-
Notifications
You must be signed in to change notification settings - Fork 769
task: add preliminary build prep action #1570
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Jeffrey Martin <jemartin@nvidia.com>
leondz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qs around minor versions, patch version naming, and manifest
| if len(version_parts) == 4: | ||
| version = ".".join(version_parts[:3]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this assumes we only ever use preX suffixes and not postX, maybe there should be a check - it's easy to imagine a scenario in which we release a postX version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version naming convention has been shifted to always be preX at the time of release. Development for new release should always move forward from Y.preX -> Y. As noted in the description further enhancement for maintaining major version branches will be need some adjustments.
| with open(file, "r") as f: | ||
| content = f.read() | ||
| with open(file, "w") as f: | ||
| new_content = content.replace(prev_version, version, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if our version ever matches the same version as anything in any of our deps, this will change that package's version, and lock it in to being changed every time in future
propose either two separate replaces, one per file or a regex-based one replacing something like {__}?version{__}? = 0.old.whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid point, I will update to more strict replacements.
| echo "version=${VERSION}" | ||
| echo "tag=v${VERSION}" | ||
| make -C docs/source cliref | ||
| git add pyproject.toml garak/__init__.py docs/source/cliref.rst |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are we checking that the manifest is broad enough to get what we need?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the three files that currently change when during a release. Happy to expand this to validate once this list is added there are no other pending changes via git and error if we find side-effects. The goal here would be to fail and avoid a possible invalid release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. It's hard to know what side-effects to look for, or if they can be identified automatically.
One strategy could be to check if everything in the repo is included in the manifest, and have a list of items that are intended to be excluded. This is possible easier to do with the fresh checkouts used here, than vs. a messy local working directory full of temp stuff.
If going this way, we should probably also report on the extent to which the package files have changed, as validation - I don't see anyone combing through a distributable's content post-release looking for spurious content, so I think the check (if made) continues to belong at packaging time if we're ever going to catch extraneous/missing content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag here would be any git reported change not already captured by this add. Printing the git status here to show impacted files should be good signal to understand and reproduce the issue.
Manually triggered workflow/pipeline for validating a commit is ready for release.
Current functionality:
releasebranch.pre1to releasedflitbuild and capture generated artifactspipinstallFuture work for next iteration:
releaseto allow for maintenance branch releasesPublishactionThe helper script
bump_version.pysimply toggles between stripping pre-release suffix and bumping patch version adding.pre1to prepare for next development cycle in files this package uses to track version based onpyproject.toml.Verification
List the steps needed to make sure this thing works