Merged
Conversation
…l changes There should be no need to use something as agressive as `git reset --hard` in a script. If such behavior is required, it should be initiated directly by the user, never in an automated fashion since it's impossible to guarantee that the workspace has no local changes which would be sacrificed to the Git gods.
We want to avoid cases where changes which are staged/gathered locally somehow impact the software that gets released - for example, allowing a local change to inadvertently get checked into the release commit. Prior to initiating the release, we will now assert that the checkout doesn't have any changes to "known files" (i.e., files which are committed to Git). Currently, this is configured to IGNORE untracked files when making this determination, though we could also choose a more aggressive option that didn't relax the assessment against untracked files. Practically speaking, an untracked file shouldn't impact the release since we have a CI pipeline responsible for the releasing. With this considered, it seemed like a reasonable trade-off since many on the team have many untracked files in their local checkouts that they use for various purposes (e.g, Configuration files, etc.) which would otherwise need to be fully stashed prior to releasing. In my experience, this change also made it a bit more tolerable to iterate on the release xtask script during development since it was a force-function that made sure I reset my changes before doing it again. The DOWNSIDE is that it becomes necessary to disable the `ensure_pristine_checkout` call, but that seems like an okay trade-off.
…ands. This corrects inconsistent behavior of the releasing script. The order that was previously dictated by the `RELEASE_CHECKLIST.md` was very intentional since the Helm chart provides the set of instructions (and versions) that `helm-docs` uses to update its README and Chart examples. The contents of the Chart also inform the output produced by `helm template` which is inserted into the `kubernetes.mdx` file by the `update_docs` function.
By checking for presence of required developer tooling ahead of making any changes, we improve the developer experience and prevent a half-finished release. Previously, the steps in the releasing process would proceed as if you had already ensured all of the required development tooling was installed on your local machine - for example, having `helm` and `helm-docs`. Therefore, during the release process, the lack of these commands present in the user's PATH would cause a failre, even though previous steps which had been completed were not idempotent. This resulted in a half-finished release which would need to be manually aborted which varied in difficulty depending on how far the release proces was able to get. For exmaple, the lack of `cargo-about` or `cargo-deny` meant that the entire run would fail after it had already changed the Cargo.toml files and swapped the CHANGELOG entries around. The instructions produced by this check also give links to README files on how to install the commands.
I don't think it makes sense to have the version pre-populated in the `NEXT_CHANGELOG.md` file prior to the release engineer making a determination about what the release will be. Practically speaking, it's necessary to evaluate whether a release has bug fixes or features prior to picking a version and the picking of a version is done by passing the appropriate paramter (patch, minor, etc.) to the release xtask script.
….md` The previous logic seemed to leave the `NEXT_CHANGELOG.md` file in a corrupted state. I've updated the regular expressions, and further changed the mechanism to not attempt to put an empty template in place within `NEXT_CHANGELOG.md` at the time that the migration occurs. Practically speaking, the template is already there, it's just placed in a way that it's an HTML comment and it's merely on the developer who introduces the first entry to the `NEXT_CHANGELOG.md` file to pick the appropriate heading for the entry they are writing. Requiring the developer be more intentional about picking the appropriate heading (e.g., "Feature", "Improvement", "Experiment", etc.) should hopefully be a fine ask and will also avoid the automated introduction of blank headings which need to be removed later. It should also reduce the almost instantaneous introduction of merge conflicts to the `NEXT_CHANGELOG.md` file.
…its. This adjusts the regular expressions we were using previously to treat the version as an opaque value, rather than `\d+.\d+.\d+`, and in some cases, uses a more defensive technique for matching them against line-endings, etc. We should aim to keep our release process flexible enough to work with pre-release versions (e.g., 1.2.3-alpha.0). The process has supported this previously, and I think that keeping that functionality is something we want to reserve. (I believe the only constraint is the regular expressions in place for these version replacements that this commit removes.)
The quotes do not seem to be getting eaten up by `helm template`'s command line parser but instead getting introduced directly to the values. This was observed both with the unnecessary introduction of quotes in the YAML strings, but (worse) when seeing that the Base64'd values for the secrets are merely Base64'd values of `"REDACTED"` rather than `REDACTED`, which seems fundamentally wrong. Happy to undo this commit if I'm missing something.
bnjjj
reviewed
Dec 29, 2022
abernix
commented
Dec 29, 2022
Co-authored-by: Jesse Rosenberger <git@jro.cc>
bnjjj
approved these changes
Dec 29, 2022
BrynCooke
approved these changes
Jan 3, 2023
abernix
added a commit
that referenced
this pull request
Jan 16, 2023
…mber Practically speaking, we can't actually guess what this version number is going to be until all the work has been collected together on `dev` and we consider the totality of the changeset and their impact on the version number. Because the previous regular expression in the release script was intentionally fragile, this also makes it less fragile since changing the template would have previously broken it. This won't get us terribly far, but it will at least be a bit more durable. Note, because regex, it's still going to be fragile, but this should give us _some_ runway. :) Follows #2323
abernix
added a commit
that referenced
this pull request
Jan 16, 2023
Today, when we land features and bug-fixes on `dev`, they require us cutting a full release before they can be tested by interested/affected parties. Thanks to work done in #2202 and #2323, we now have just enough automation in our release pipeline to take care of some of the necessary work for building these releases. With some follow-up work, we can have these triggered through: - [Manual triggers] through CircleCI - [Scheduled pipeline] runs in CircleCI [Scheduled pipeline]: https://circleci.com/docs/scheduled-pipelines/ [Manual triggers]: https://circleci.com/docs/triggers-overview/#run-a-pipeline-from-the-circleci-web-app This work touches on ideas from #229 and #242, though neither of those issues are _directly_ "build nightly releases", though I did suggest it in my comments on one of them.
7 tasks
abernix
added a commit
that referenced
this pull request
Jan 19, 2023
…mber (#2408) Practically speaking, we can't actually guess what this version number is going to be until all the work has been collected together on `dev` and we consider the totality of the changeset and their impact on the version number. Because the previous regular expression in the release script was intentionally fragile, this also makes it less fragile since changing the template would have previously broken it. This won't get us terribly far, but it will at least be a bit more durable. Note, because regex, it's still going to be fragile, but this should give us _some_ runway. :) Follows #2323
abernix
added a commit
that referenced
this pull request
Jan 20, 2023
Today, when we land features and bug-fixes on `dev`, they require us cutting a full release before they can be tested by interested/affected parties. Thanks to work done in #2202 and #2323, we now have just enough automation in our release pipeline to take care of some of the necessary work for building these releases. With this work, we can have these triggered through: - [Manual triggers] through CircleCI - [Scheduled pipeline] runs in CircleCI [Scheduled pipeline]: https://circleci.com/docs/scheduled-pipelines/ [Manual triggers]: https://circleci.com/docs/triggers-overview/#run-a-pipeline-from-the-circleci-web-app This touches on ideas from #229 and #242, though neither of those issues are _directly_ "build nightly releases", though I did suggest nightlies in my comments on one of them. Overall, this creates Router versions of the format `v0.0.0-nightly+YYYYMMDD.COMMIT_HASH`, which look roughly like `router-v0.0.0-nightly+20220102.abcd1234-x86_64-unknown-linux-gnu.tar.gz` as binaries on CircleCI Artifacts. In the future, we can attach these to GitHub Releases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR follows-up #2202 and it consists of several commits which can stand alone, if necessary. Each of those commits has their own message and while I suggest reviewing the totality of the PR, it's worth considering the text of the individual commit messages for additional context on the changes.
As a summary of those commits:
git reset --hardcommand which destroyed my local changeshelm-docsandhelm templatecommands.NEXT_CHANGELOG.mdNEXT_CHANGELOG.mdentries toCHANGELOG.mdhelm template's--setflagsContributes to #2261