neil: store version data in project deps.edn

[borkdude]

Neil version discussion from: #81

[borkdude]

Sub-topic:

We currently store project data (the name and in the future, the version) in deps.edn.
But storing this in deps.edn doesn't make it accessible once the project is deployed. Or can we still read it from the clojure.basis System property? Having this file on the classpath seems more robust.

[rads]

There's a strong convention of putting project metadata in a top-level file such as project.clj or package.json. All kinds of Clojure projects may need to manage versions, but it seems like reading back the version at runtime is more of a CLI-specific thing. I think following the convention established by Leiningen and NPM is more important than being able to read the version from the classpath.

For neil's case where we do need to print the version, there are (unfortunately) a lot of options, depending on how neil is used:

• Local dev: Read deps.edn with slurp
• Script usage: Read deps.edn and include this data in bb gen-script
• JAR usage: Read deps.edn and include this data in the target dir as part of build.clj
• Gitlibs usage: Read deps.edn during clj -X:deps prep (see docs)
  • Not a great option since this is another manual step from the user before being able to use the lib

I do appreciate the simplicity of being able to use io/resource to avoid any build steps. That said, I'm not sure if it's worth the ugliness of resources/META-INF/org/lib/project.edn when people are used to seeing the same info in a project.clj file already.

Disclaimer: This is just my opinion and I'm fine with either choice in the end. It may be worth sharing this thread with more people so we can get more community feedback.

[borkdude]

@rads I think you're right. Let's continue with the current approach of storing the data in deps.edn.

[borkdude]

Sub-topic: version format {:major .. minor .. patch ..} vs "1.0.0".

I wonder if we should store the version attributes as strings (since perhaps people will use different things) or even as a single string, as this allows people to be more flexible. E.g. clj-kondo uses 2022.08.01 as the version format and clojure-lsp even uses a more elaborate date-based format. If we would have neil version set <...> then people could choose any date format they like. But for most projects, the major/minor/patch format probably works and even for clj-kondo, I think I could use 2022.8.1 instead of the leading zeros. There's also benefit of having consistency across projects.

Should the neil version patch command be more explicit: neil version patch inc and neil version patch set ... perhaps? Maybe I'm overthinking and having the same command set as npm is beneficial.

Storing the version as a single string has the benefit that you don't need any other libraries to pull it out and use it e.g. in build.clj. Storing the single string is also what other tools like mvn and npm seem to be doing. Maybe we should do that too and parse the version: if it's unparsable as major/minor/patch then users just won't be able to use our neil version patch commands and they are on their own (they could edit the version manually).

[rads]

I wonder if we should store the version attributes as strings

I think the SemVer strings are easier to read and write for humans, so I'm in favor of this. We can go between the map and string representations as long as the string is an expected format such as "1.0.0". I don't think the flexibility is different one way or the other since there's not much neil can do to help with an unknown format, regardless of whether it's an unstructured string or a map with unknown keys.

Should the neil version patch command be more explicit: neil version patch inc and neil version patch set ... perhaps? Maybe I'm overthinking and having the same command set as npm is beneficial.

• neil version patch inc seems redundant to me because we already have neil version patch which lines up with the npm version patch command

• neil version patch set is already implemented as neil version patch 42 (though this is not part of npm's behavior)

• NPM also supports npm version <newversion> to manually set the version, which would be easy enough to implement on the neil side as well

  $ npm version 2022.8.1
  v2022.8.1
  
  $ cat package.json
  {
    "version": "2022.8.1"
  }
  

Another option to consider would be to support both formats:

# Equivalent commands
$ neil version patch 42
$ neil version patch set 42

# Equivalent commands
$ neil version patch
$ neil version patch inc

This way someone familiar with NPM can get started no problem, but we also have the option of using the more verbose API if we want. This wouldn't be a problem for CLI parsing since set and inc are not valid values for [major|minor|patch] (technically qualifier could be anything but I think disallowing set as a qualifier is a reasonable compromise).

If we had to choose just one API, I would still go with NPM since that's an established interface used by tons of people already.

Maybe we should do that too and parse the version: if it's unparsable as major/minor/patch then users just won't be able to use our neil version patch commands and they are on their own (they could edit the version manually).

Yes, I think this makes sense given the above neil version <newversion> command to set the version manually. Using SemVer syntax is a reasonable ask for using the neil version [major|minor|patch] commands.

[borkdude]

@rads I'm having second thoughts about the automatic tagging, e.g. for the following use case:

• user wants to update version indeps.edn
• then wants to extract the version to update a file in the project so it can print its own version`
• then wants to create a git tag based on the version

[rads]

Updated the example above to get rid of the alias... we can just use neil version 🙂

[rads]

Note that the example above would almost work for NPM if you simply replaced neil with npm, which I think is pretty cool. The only difference is that $(npm version) returns a JSON object (not just one version) so you'd have to extract the project version with jq or jet.

[rads]

On that note, maybe neil version should return a map to align with NPM.

$ neil version
{:neil "0.1.43"
 :project "0.1.1"}

[borkdude]

I think having a separate neil version tag command makes most sense. Why complect things too much. We don't have to be 100% compatible with npm.

[rads]

@borkdude: Just want to repeat this since I've been thinking about it some more now that we just return strings for the versions: What do you think of changing neil version to return both the :neil version and the :project version? That way there is no confusion about which version we're talking about and it's consistent with NPM's behavior.

$ neil version
{:neil "0.1.43"
 :project "0.1.1"}

[rads]

@borkdude: Based on the discussions above, here's what I'm thinking for next steps. The "Confirmed" items seem pretty safe to get started on since I can work on the implementation while we finalize the CLI. The items in "To Be Confirmed" are more about the CLI itself which is why I wanted to double-check with you first.

Regardless, it will be a little while until I can start on the work this evening, so let me know if there's anything you'd like to change in the plan below in the mean time. I'll make sure to address any feedback before I get started.

• Confirmed: Implement neil version set <new-version>
  • Explicitly set the :version to <new-version>
  • The <new-version> can be any string that's not already a command (e.g. [major|minor|patch|tag])
  • This could be aliased or renamed to neil version set <new-version>
    • Moving forward with neil version set, not implementing neil version <new-version>
• Confirmed: Implement neil version tag
  • Runs git commit -m <current-version>
  • Runs git tag <current-version>

  $ neil version patch --no-tag
  $ bb gen-script
  $ git add .
  $ neil version tag
  

• To Be Confirmed: Add --tag false/--no-tag as an alias for --no-git-tag-version
  • The shorter version is easier to type and read
  • It's easy enough to support both here to maintain compatibility with NPM
• To Be Confirmed: Add aliases or rename commands to be more explicit
  • My personal preference would be adding new aliases instead of breaking NPM compatibility
  • I can move forward with either approach once I have confirmation on what we want to do

  neil version patch set 42
  neil version minor inc
  neil version set 2022.8.1
  

[rads]

I removed the --force option from neil tag in my comment above. Once I thought about it some more, I realized we'd always have a dirty working directory in the example. It seems more intuitive for neil tag to not care about dirty or clean working directories and just try to git commit and git tag regardless.

[borkdude]

neil version

I would go for neil version set <new-version> to not have any ambiguity with the other subcommands and not do the neil version <...> one.

[borkdude]

Sounds good to me

On Wed, 10 Aug 2022 at 07:33, Radford Smith ***@***.***> wrote: @borkdude <https://github.com/borkdude>: Just want to repeat this since I've been thinking about it some more now that we just return strings for the versions: What do you think of changing neil version to return both the :neil version and the :project version? That way there is no confusion about which version we're talking about and it's consistent with NPM's behavior. $ neil version {:neil "0.1.43" :project "0.1.1"} — Reply to this email directly, view it on GitHub <#90 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACFSBVJCBQXWJWR6KZV6HLVYM5MNANCNFSM554T4HAA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- https://www.michielborkent.nl https://www.eetvoorjeleven.nu