ENH: allow in-script tagging in ioc-deploy #200
Conversation
|
The diff here still shows the variant from before the other PR was merged, I'll see if closing and re-opening fixes it |
|
It worked, now with the updated diff I'll have an easier time writing the description and the reviews will be simpler later too. |
|
This is likely the last ioc-deploy PR for a while unless new bugs are found |
There was a problem hiding this comment.
This looks good to me, a few nitpicks.
I don't think we have any restrictions on who can make tags, and if that's the case this seems fine. Though it does open us up to people messing up the semantic versioning convention and pushing annoying tag names. I don't quite think this is the place to enforce that, and attempting to do so would probably result in a huge annoying block of info.
scripts/ioc_deploy.py
Outdated
| ) | ||
| except subprocess.CalledProcessError as exc: | ||
| raise ValueError( | ||
| f"Error cloning {github_org}/{name}, please make sure you have the correct access rights and the repository exists." |
There was a problem hiding this comment.
I'll split this up, I'm getting away with long strings because ruff doesn't care
There was a problem hiding this comment.
While I was at it, I split up the other excessively long lines in the file
Rather than aiming for any specific length I tried to separate them by semantically logical bits
scripts/ioc_deploy.py
Outdated
| except KeyboardInterrupt: | ||
| logger.error("Interruped with ctrl+C") | ||
| logger.debug("Traceback", exc_info=True) | ||
| rval = ReturnCode.EXCEPTION |
There was a problem hiding this comment.
This seems more like a ReturnCode.NO_CONFIRM case to me, where if a user quits out it was intentional
| """ | ||
| Return the most recent commit message or raise a subprocess.CalledProcessError | ||
| """ | ||
| cmd = ["git", "log", "-1"] |
There was a problem hiding this comment.
| cmd = ["git", "log", "-1"] | |
| cmd = ["git", "log", "-1", "--pretty=%B"] |
For me this gets all the nitty gritty from the log output as well, adding a few arguments can strip that
There was a problem hiding this comment.
This doesn't work for a shallow clone and I do not know why
There was a problem hiding this comment.
Weird, this works for me in a clone --depth 1, maybe I'm doing something differently?
Details
$ git clone --depth 1 [email protected]:tangkong/hello-world
Cloning into 'hello-world'...
remote: Enumerating objects: 3, done.
remote: Counting objects: 100% (3/3), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 3 (delta 0), reused 3 (delta 0), pack-reused 0 (from 0)
Receiving objects: 100% (3/3), done.
$ cd hello-world/
$ git log
commit e2e299826a1f0d96bf1f2e5448d33032f85157f2 (grafted, HEAD -> master, tag: v0.0.1, origin/master, origin/HEAD)
Author: tangkong <[email protected]>
Date: Fri Jan 12 18:17:25 2018 +0000
Merge pull request #1 from tangkong/readme-edits
Update1 approved
$ git log -1 --pretty=%B
Merge pull request #1 from tangkong/readme-edits
Update1 approvedThere was a problem hiding this comment.
I swear this was failing for me last week but now I see the same
There was a problem hiding this comment.
I after seeing it fail (somehow) last week and switching to the full log, I ended up really appreciating seeing the full thing including the timestamp, user, etc., so it's more obvious if you are accidentally tagging prior to pushing or merging the PR.
There was a problem hiding this comment.
So to that end I'm going to change the docstring instead of the command
There was a problem hiding this comment.
The only thing a user might not care to see is the commit hash, but then again I personally wouldn't mind it. Sounds good to me
|
One more re-check from me that the main code paths work after the minor adjustments and then I'm clicking merge! |
Description
Main goal: allow us to make releases from the command line without going to the github website, for convenience
Main changes:
Other things to note:
KeyboardInterrupt, since it's way more likely to happen when the user is dropped into a text prompt, and seeing a traceback when you do it is a bit annoying.--auto-confirm, the tag will be made automatically with no message.I also snuck some smaller changes into here
Motivation and Context
https://jira.slac.stanford.edu/browse/ECS-6035
For releasing hutch-specific templated IOCs that amount to version bumps, etc., people have expressed that they find it annoying to open up github and do by hand what in the past they had done all in the command-line, so I provide these features here.
This isn't so necessary or useful for when you're already on github doing a code review process.
How Has This Been Tested?
Interactively only
Where Has This Been Documented?
Added to the help text and readme