Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump (set) version to 1.0.0 #287

Merged
merged 2 commits into from
Apr 5, 2020
Merged

Bump (set) version to 1.0.0 #287

merged 2 commits into from
Apr 5, 2020

Conversation

kmsquire
Copy link
Contributor

@kmsquire kmsquire commented Jun 3, 2019

@TotalVerb @ararslan, okay with this?

Cc: @davidanthoff
Closes #283.

@fredrikekre
Copy link
Contributor

This will not solve anything; rather it will make the 180 or so packages that depend on JSON uninstallable if JSON is already installed.

@quinnj
Copy link
Member

quinnj commented Jun 3, 2019

Won't those packages just stay on JSON.jl's current release? I think we'd only get in trouble once a dependent package says they only accept JSON 1.0 and then causes incompatibilities between all the other packages that haven't upgraded yet, right?

@davidanthoff
Copy link
Collaborator

This will not solve anything; rather it will make the 180 or so packages that depend on JSON uninstallable if JSON is already installed.

By that logic we can't tag any new minor version going forward, correct?

As far as I understand the current situation, tagging a new minor release pre 1.0 and tagging a 1.0.0 has exactly the same effect on downstream packages. But at least once we made the transition to 1.0.0, one can more easily tag minor versions without wreaking havoc with the downstream ecosystem.

We probably need an equivalent to JuliaRegistries/General#718 for this PR here? And then PRs against all depending packages that adjust the compat bounds in their Project.tomls, right? Quite a pain, but on the other hand probably unavoidable at some point, so it seems better to me to fix this now rather than waiting until even more packages depend on JSON.

@fredrikekre
Copy link
Contributor

if JSON is already installed.

^ If you have JSON 1.0 and try to add a package that is not compatible with it Pkg will not downgrade it for you, leading to a cryptic "unsatisfiable requirement" errors and 10 Discourse posts.

Why can't 1.0 be the next breaking release instead?

@fredrikekre
Copy link
Contributor

fredrikekre commented Jun 3, 2019

Why can't you just say

[compat]
JSON = "0.20"

?

@ararslan
Copy link
Member

ararslan commented Jun 6, 2019

I don't really feel strongly about this, but Fredrik makes some good points. I think it would make sense to wait to declare 1.0 until we disruptive changes to introduce. I don't see an urgent need to declare 1.0 right away anyway.

@stevengj
Copy link
Member

I agree with @davidanthoff, here. Until we tag 1.0, there is no way to indicate that packages are compatible with minor releases. So, for example, if I put JSON = "0.20" in my Project.toml as suggested above, then it will prevent JSON from being updated to 0.21.

So isn't tagging 1.0 exactly as disruptive as tagging the next minor release, 0.22?

@fredrikekre
Copy link
Contributor

But 0.22 should not be tagged unless it is breaking so when JSON is ready for a breaking release it can be 1.0.0.

@stevengj
Copy link
Member

stevengj commented Jan 7, 2020

What version number would you use if you add a new feature? That is not breaking, and would normally be a minor version bump.

@fredrikekre
Copy link
Contributor

Right, so if you have not released a major version you only have 2 numbers to play with. The first one (minor) is for signaling breaking changes so you will have to increase the patch number for both bugfixes and features. I believe this is the strategy most package have adopted now.

@TotalVerb
Copy link
Collaborator

I am feeling somewhat unsure about bumping the patch version when features are added, because this defeats the purpose of semantic versioning. This package does not get new features particularly often, but I would support a 1.0.0 when that does occur.

@KristofferC
Copy link
Member

I am feeling somewhat unsure about bumping the patch version when features are added, because this defeats the purpose of semantic versioning.

Pre 1.0, semantic versioning is "undefined". In Julia it means that minor versions are breaking. So tagging a new minor version pre 1.0 will not automatically be updated to packages that depend on JSON when they use the recommended compat bounds.

@TotalVerb
Copy link
Collaborator

I do understand this convention, but it is unfortunate that 0.21.1 is ambiguously either a fix for an obscure edge case in float parsing or brand new infrastructure for the reader (which happens not to break the package's very limited interface). This to me defeats the purpose of semantic versioning and I feel like a move to 1.0.0 even without breaking changes is appropriate.

Node.js solves the conflicting dependency problem by allowing multiple versions of a package simultaneously. Is there a reason Pkg does not allow this?

@TotalVerb
Copy link
Collaborator

Another option is that, if we know 1.0.0 is in fact not a breaking release, we write a script to PR the GeneralRegistry to declare the appropriate [compat] for all registerer packages.

@ararslan
Copy link
Member

ararslan commented Jan 8, 2020

Is there a reason Pkg does not allow this?

It does with environments: each environment can have a separate version of a package. For example, you can install JSON 0.x "globally" but do julia --project=whatever and get JSON 0.y if the project has set its JSON requirement as such.

@TotalVerb
Copy link
Collaborator

TotalVerb commented Jan 8, 2020

However, if A depends on B, and both depend on conflicting versions of JSON, then there will be an unsatisfiable dependencies error. Node.js allows multiple versions of the same dependency to be installed within the same environment.

But, to answer my own question, I just read https://lexi-lambda.github.io/blog/2016/08/24/understanding-the-npm-dependency-model/ which provides several compelling arguments against allowing duplicate dependencies, especially in a non-ducktyped language.

@KristofferC
Copy link
Member

Julia's code loading itself doesn't currently allow this and for a good reason imo. It would be confusing if you do e.g.

using DataFrames
df = DataFrame(...)
df2 = CSV.parse("foo.csv")

and df, df2 would both be DataFrames but from a different version of DataFrames.jl.

@davidanthoff
Copy link
Collaborator

On the other hand, there are many situations where it wouldn't be a problem and it would solve a real need. For example, in our VS Code extension we inject some code into the REPL process to communicate with the JavaScript VS Code extension. It would be great if we could use a specific version of JSON in that code, without making that now the version that users of the REPL have to use. There wouldn't be any danger of duplicate types with different versions because our use of JSON would be purely internal, for the implementation of the communication between our code that runs in the REPL process and the Javascript extension code. I have very many other examples like that.

I feel there is probably some in-the-middle design space where a package could declare that it wants to take a "private" dependency on some other package. Maybe the rule then is that you can't re-export any type from that private dependency, or something like that? Or maybe that is just a guideline. In many ways I feel that Julia allows you to shoot yourself in the foot in so many ways already that adding one more wouldn't really be a problem, if it helps address some real other problems :)

@codecov
Copy link

codecov bot commented Jan 8, 2020

Codecov Report

Merging #287 into master will increase coverage by 3.15%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #287      +/-   ##
==========================================
+ Coverage   95.06%   98.21%   +3.15%     
==========================================
  Files           8        8              
  Lines         405      392      -13     
==========================================
  Hits          385      385              
+ Misses         20        7      -13
Impacted Files Coverage Δ
src/Parser.jl 99.01% <100%> (+2.36%) ⬆️
src/Writer.jl 95.6% <100%> (+2.05%) ⬆️
src/specialized.jl 100% <0%> (+7.89%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0547ffc...7b0cfc3. Read the comment docs.

Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking, I am okay with this. I think the benefits of post-1.0 versioning (i.e. actual semver) outweighs the costs of a "breaking" version bump.

@stevengj
Copy link
Member

stevengj commented Apr 5, 2020

Can this be merged?

@TotalVerb TotalVerb merged commit 55f8500 into master Apr 5, 2020
@TotalVerb TotalVerb deleted the kms/version-1.0.0 branch April 5, 2020 18:37
@stevengj
Copy link
Member

stevengj commented Apr 6, 2020

It would also be nice to submit PRs to packages that use JSON.jl to have them list JSON 1 in their allowed versions. Is there an easy way to get a list of packages that require JSON?

@TotalVerb
Copy link
Collaborator

TotalVerb commented Apr 6, 2020

~/.julia/registries/General> grep -r 'JSON =' | cut -d/ -f2 | sort | uniq
AccurateArithmetic
AdventOfCode
Arena
AssetRegistry
Atom
AWSCore
Azure
BeaData
BenchmarkCI
BenchmarkTools
BIDSTools
BigArrays
BinaryBuilder
BioEnergeticFoodWebs
BioServices
Blink
BlsData
Bukdu
Caesar
CancerImagingArchive
CAOS
CategoricalArrays
Catlab
CloudGraphs
ColorBrewer
CompatHelper
Compose
Conda
Config
Coverage
CSSUtil
Currencies
CurricularAnalytics
CUTEst
D3Trees
D3TypeTrees
DarkSky
Dashboards
DataDepsGenerators
DataVoyager
DBnomics
DeIdentification
DemoCards
Dhall
Diana
DispatcherCache
DistributedFactorGraphs
DocumentationGenerator
Documenter
DrakeVisualizer
DropboxSDK
DumbCompleter
ECharts
Electron
ElectronDisplay
Emoji_Entities
EMpht
EnglishText
Exercism
ExpressPathToRegex
Figures
Firestore
FlashWeave
FredApi
FredData
FreeParameters
FwiFlow
Gadfly
GaloisFields
GasModels
GasPowerModels
GBIF
GCP
Genie
GeoJSON
GeoMakie
GetGene
GigaSOM
GitHub
Glo
GlobalSearchRegressionGUI
GoogleCloud
GoogleCloudObjectStores
GoogleCodeSearch
GoogleMaps
Gridap
HackerNews
Harlequin
Homebrew
IJulia
IMFData
InfrastructureSystems
Interact
InteractBase
Joseki
JqData
JSCall
JSExpr
JsonGrinder
JSONSchema
JSONWebTokens
JuliaRunClient
JuliaWebAPI
JuLIP
Juniper
JupyterParameters
JWTs
KCenters
Knockout
Kuber
Languages
LanguageServer
LazyJSON
Literate
Lyra
Mads
MakieGallery
Mangal
MathOptFormat
MathOptInterface
Matte
Memento
Merly
MeshCat
Mill
MIMEBundles
Mimi
MirrorUpdater
MLJBase
Modia
Modia3D
MXNet
Namtso
NBInclude
NCEI
Neo4j
NoveltyColors
OMETIFF
OnlinePackage
OpenIDConnect
OpenStreetMapX
ORCA
Pages
Pandoc
PhaseSpaceIO
PhilipsHue
PkgBenchmark
PkgDev
Plotly
PlotlyBase
PlotlyJS
Plots
Pluto
Polymake
PowerDynamics
PowerModels
PowerModelsDistribution
PowerModelsSecurityConstrained
PowerSimulations
PowerSystems
Presentation
PropDicts
Pushover
RealNeuralNetworks
REDCap
Reddit
Registrator
RegistryCI
Remark
RemoveLFS
RepoSnapshots
Reproduce
SalesForceBulkApi
Scryfall
SDDP
SearchLight
SecretSanta
SemanticModels
SimilaritySearch
SimpleHypergraphs
Slack
Slacker
SMM
SolidStateDetectors
SolverBenchmark
StackOverflow
StaticLint
StructDatabaseMapping
Swagger
SyntheticGrids
TableSchema
TableShowUtils
TableView
Telegrambot
Temporal
TextAnalysis
TextSearch
Transformers
Turf
TwilioSMS
Twitter
Unmarshal
Vega
VegaDatasets
VegaLite
WaterModels
Weave
WebIO
WeightedArrays
WorldBankData
XLSXasJSON
YahooFinance
Zabbix
Zarr

Unfortunately, I'm not sure where to go from here. I am not too familiar with the GitHub API and how to make mass PRs.

@davidanthoff
Copy link
Collaborator

Ideally these packages have CompatHelper installed and will automatically get a PR that bumps their version. I think the bigger question is whether we should relax the bounds in the registry for all these packages...

@KristofferC
Copy link
Member

KristofferC commented Jun 30, 2020

A lot of packages are still bound to pre 1.0. I still feel this was completely pointless. Gratz, there is a 1.0 now that no one uses and any fixes on top of it (like 4aad0c8) doesn't reach anyone. What were the benefits again?

@quinnj
Copy link
Member

quinnj commented Jun 30, 2020

Perhaps 4aad0c8 should be backported and a new 0.X.Y release should be made then; that's common practice when dependencies are in a transition phase to a new major release.

@KristofferC
Copy link
Member

Yeah, it would be great if the people arguing for this PR, not minding the extra maintenance burden, would help with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tag a 1.0.0
8 participants