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

Implement tag command #525

Open
dom96 opened this issue Aug 21, 2018 · 22 comments
Open

Implement tag command #525

dom96 opened this issue Aug 21, 2018 · 22 comments

Comments

@dom96
Copy link
Collaborator

dom96 commented Aug 21, 2018

Usage: nimble tag v0.1.1

Nimble will run git tag v0.1.1 (or equivalent) and verify that the package's .nimble file has been updated with that version.

Bonus feature:

  • Enforce semantic versioning by checking for breaking API changes.
@dom96 dom96 mentioned this issue Apr 9, 2019
@dom96 dom96 added this to the 0.11.0 milestone Sep 21, 2019
@zevv
Copy link
Contributor

zevv commented Sep 21, 2019

I guess having nimble editing the .nimble file is a no-no? It would be convenient if one command would do all the bookkeeping needed for a release - updating nimble,making the tag and pushing the repo all in one go.

@PMunch
Copy link

PMunch commented Sep 22, 2019

Problem with that though is that the nimble file is NimScript and IIRC you can do things like version = staticExec("git describe --tags") so the logic for what to change might be a bit difficult to get right..

@kristianmandrup
Copy link

Let's add this command feature ASAP. All other package libraries I know of do this sort of tagging as part of publish. We could even add the following convenience commands:

$ nimble bump patch # increase version by `0.0.1` and run `nimble tag`
$ nimble bump minor # increase version by `0.1.0` and set patch version `0`, then run `nimble tag`
$ nimble bump major # increase version by `1.0.0` and set patch + minor version `0`, and run `nimble tag`

I'll give it a go

@kristianmandrup
Copy link

I added initial support for the tag command here. Currently only supports git

Tag repo and push tags to remote repo

$ nimble --tag 0.1.0

Increase patch version tag and publish (ie. 0.1.2 to 0.1.3)

$ nimble publish --tag patch
# shorthand alternative
$ nimble publish --tag .

Increase minor version tag and publish (ie. 0.1.2 to 0.2.0)

$ nimble publish --tag minor
# shorthand alternative
$ nimble publish --tag m

Increase major version tag and publish (ie. 0.1.2 to 1.0.0)

$ nimble publish --tag major
# shorthand alternative
$ nimble publish --tag M

The increase tag variant works for the tag command as well:

$ nimble tag --tag M

Perhaps the option should be renamed version though

@kristianmandrup
Copy link

Now uses bump under the covers.
Could use some optimisation, leveraging bump way more as it is super powerful

@genotrance
Copy link
Contributor

Main feedback is that you cannot depend on bump as a package since nimble is the package manager and you cannot install bump before nimble is built.

https://github.com/nim-lang/Nim/blob/devel/koch.nim#L163

@dom96
Copy link
Collaborator Author

dom96 commented Jan 27, 2020

At this point, why don't we just make nimble bump an alias to bump? Better yet, Nimble should have the ability to delegate functionality to executables named nimble-<command_name> so bump would just need to offer a nimble-bump binary.

@kristianmandrup
Copy link

kristianmandrup commented Jan 28, 2020

We could just execute bump in the shell if nimble is provided bump arguments or perhaps a bump subcommand?

nimble publish --bump patch

Would make it much easier to implement and stay in sync with bump functionality

@kristianmandrup
Copy link

kristianmandrup commented Jan 28, 2020

I've implemented a new version of this feature where I've removed the dependency on bump.
Made a number of additional improvements. Pls check tag-command branch or latest commits on the branch

Needs testing, unit testing (tmrw?) and review. Likely the options format need to be agreed on for optimal UX

Should I open a PR to work from?

@dom96
Copy link
Collaborator Author

dom96 commented Jan 28, 2020

We could just execute bump in the shell if nimble is provided bump arguments or perhaps a bump subcommand?

Yes, again, I'd like a generic solution where packages can provide binaries named nimble-<name> and then Nimble can execute them via nimble <name>. That way the bump package could provide a nimble-bump package and interoperate effortlessly, the user would just need to install it (we can give hints for this for some popular functionality)

@Araq
Copy link
Member

Araq commented Jan 30, 2020

named nimble-<command_name> so bump would just need to offer a nimble-bump binary.

Please don't. Either make "bump" a real feature of Nimble or stay out of bump's way. "Every command must start with nimble-" doesn't add any usability whatsoever.

@kristianmandrup
Copy link

@Araq I also recommend keeping nimble dependency free. My latest commit has no dependencies.
bump can be used on the side. A reference + note about bump could be added to the nimble Readme perhaps. Just my thoughts. Cheers.

@dom96
Copy link
Collaborator Author

dom96 commented Feb 4, 2020

"Every command must start with nimble-" doesn't add any usability whatsoever.

Huh? It obviously adds usability, it would make these kinds of features trivial.

@genotrance
Copy link
Contributor

@dom96 - main push back from me is that nimble bump is longer than bump. Having a generic feature in nimble would be interesting if there were many such external sub-commands. git does something like this right?

Options:

  • Just ship bump in the Nim distro
  • Merge bump code into nimble
  • New feature in nimble where nimble xyz checks findExe("nimble-xyz") and then calls that binary

cc @disruptek

@disruptek
Copy link

I don't really have much of an opinion, except that it seems completely irrelevant to the compiler and probably even orthogonal to the distribution. Pulling it into nimble would allow me to deprecate it as a separate tool, so that would be nice.

I think dynamic subcommands are a cute feature, but only if the parent and child apps actually benefit from the integration. I don't really see that here, but maybe you guys have a good idea that hasn't occurred to me.

For example, bump functions will get absorbed into nimph so that

  • github releases get fully integrated
  • version bumps don't happen when the repo is dirty
  • bumps can safely handle arbitrary branches, detached heads, etc.
  • rewrites of .nimble files can precede more rigorous tests
  • changelogs can be inferred or generated automagically

...and so on...

@disruptek
Copy link

As I said in IRC, I'm flattered that Kristian took the time on this PR. I also think the fact that his time is now costing us time and creating pushback is absurd.

Let's close this PR with apologies. I agree with @dom96 that automatic subcommand extensions makes sense. It lets subcommands develop independently, removes friction from the ecosystem, and stifles duplication of effort like this.

Let's also make a way to query nimble for its available subcommands and include their (abbreviated) --help output in nimble's. Maybe we can also create a tag for nimble subcommands so that they are more easily found via a nimble search. Then we can advertise that in the software and online, somewhere, too.

@kristianmandrup
Copy link

Thanks for the thumbs up. Please know that the PR has not been fully tested and is mainly a POC. I'm still very new to the Nim ecosystem, so perhaps better that someone take charge, test + polish it, to finalise this PR. How about the CLI arguments? should it be --bump patch or perhaps use tag or version?

@dom96
Copy link
Collaborator Author

dom96 commented Feb 6, 2020

@dom96 - main push back from me is that nimble bump is longer than bump

This push back doesn't make sense to me. If nimble bump works (by calling nimble-bump from the bump package) then executing bump will work too since the Nimble package will continue to expose it as its own binary (i.e. you'll have two binaries, nimble-bump and bump), so the user can use it directly too if they wish.

@kristianmandrup
Copy link

I think it would be a great idea to integrate bump with nimble. The main question is how to do this and how to provide the best user experience.

Sub-commands would likely be the way forward, as it opens up more flexibility. Perhaps some of the other commands should use sub-commands as well?

In terms of requiring pre-existing installation of bump that would make it much easier to do obviously and bump could then evolve independently. Let's try it. Much easier pathway than building new functionality or integrating bump directly into the core of nimble. Your thoughts?

@genotrance
Copy link
Contributor

nimble bump simply calling nimble-bump would simplify the design within Nimble but won't make the user experience any better. If designed generically, nimble won't know that nimble pump is dumb or that the user needs to do something for nimble bump to work (i.e. nimble install bump).

If nimble instead maintains a list of known subcommands, when a user runs nimble bump, nimble would know that it needs to download the bump package if not already installed. Then the user experience would be seamless and this effort would be worthwhile.

So no need to changes bump to add a nimble-bump target since nimble would know that nimble bump should findExe("bump") and nimble install bump. And once this capability is enabled, adding new sub-commands would basically need a line of code pointing to the package and corresponding binary name.

Next question is when nimble should update an existing install of bump to the latest.

@dom96
Copy link
Collaborator Author

dom96 commented Feb 7, 2020

Much easier pathway than building new functionality or integrating bump directly into the core of nimble. Your thoughts?

If that question is aimed at me then you should already know my answer: yes, I would prefer to keep Nimble simpler and allow its evolution via external commands.

If nimble instead maintains a list of known subcommands, when a user runs nimble bump, nimble would know that it needs to download the bump package if not already installed. Then the user experience would be seamless and this effort would be worthwhile.

This approach doesn't scale. One thing we can do is mark packages providing a binary that Nimble can call with a nimble-command-<cmd_name>. Then when Nimble is executed with a command it does not understand it can look for packages with those tags and list them as candidates for installation.

But this is something that can come after, it's an additional feature that makes discovery better, but it's not necessary. Let's focus on the basics first. For a basic implementation I think a nimble- prefix to the binary is a must, it will also allow commands to be run faster than having to look up the installed packages (when we do implement the tagging logic I described above).

Next question is when nimble should update an existing install of bump to the latest.

I don't think we should worry about this for now. This is another feature that is much further in the future and we can cross that bridge when we come to it.

@genotrance
Copy link
Contributor

Related to #433 which is the generic requirement.

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

No branches or pull requests

7 participants