[Breaking] Rename cas executable to nativelink #573
Conversation
There was a problem hiding this comment.
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image, ubuntu-20.04, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @allada and @MarcusSorealheis)
There was a problem hiding this comment.
Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Remote / large-ubuntu-22.04 (waiting on @allada and @MarcusSorealheis)
a discussion (no related file):
Since this is a breaking change, should we tally an entry into https://github.com/TraceMachina/nativelink/blob/main/CHANGELOG.md now or mark the issue tracker in a way such that next release it is easy to find what breaking changes have been made? Git commit titles work for me, maybe we should codify that (or alternatives) in https://github.com/TraceMachina/nativelink/blob/main/CONTRIBUTING.md
As for CHANGELOG.md format, do we need gitsha or just reference ticket id and traverse from there to commit? Personally I do like easy access to gitsha, but also seems like an additional thing to manage in terms of updating a changelog file.
aaronmondal
force-pushed
the
rename-cas-to-nativelink
branch
from
bc42a01
to
874a815
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), publish-image, ubuntu-20.04, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @allada and @MarcusSorealheis)
a discussion (no related file):
Previously, adam-singer (Adam Singer) wrote…
Since this is a breaking change, should we tally an entry into https://github.com/TraceMachina/nativelink/blob/main/CHANGELOG.md now or mark the issue tracker in a way such that next release it is easy to find what breaking changes have been made? Git commit titles work for me, maybe we should codify that (or alternatives) in https://github.com/TraceMachina/nativelink/blob/main/CONTRIBUTING.md
As for
CHANGELOG.mdformat, do we need gitsha or just reference ticket id and traverse from there to commit? Personally I do like easy access to gitsha, but also seems like an additional thing to manage in terms of updating a changelog file.
I think that's a good thing in general but it's not clear to me what a good balance between keeping the changelog updated and small commits is.
Technically we only need to update the changelog on every tag. So we don't have to always keep it updated. I think I'm leaning more towards only bumping the changelog on commits that are intended to be tagged commits.
There was a problem hiding this comment.
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), publish-image, ubuntu-20.04, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @allada and @MarcusSorealheis)
a discussion (no related file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
I think that's a good thing in general but it's not clear to me what a good balance between keeping the changelog updated and small commits is.
Technically we only need to update the changelog on every tag. So we don't have to always keep it updated. I think I'm leaning more towards only bumping the changelog on commits that are intended to be tagged commits.
Looking into this we'll probably have to make some changes to the existing changelog anyways. What we're currently doing is more or less a copy of the git history. That's explicitly not what we're supposed to do for the changelog. Instead, it should be a short, concise, human-readable thing that only contains relevant information on CVEs, migration paths and major changes.
See: https://github.com/coreinfrastructure/best-practices-badge/blob/main/docs/criteria.md#change-control
There was a problem hiding this comment.
@allada Is this safe to merge without breaking our existing deployments?
Reviewable status: 0 of 2 LGTMs obtained, and pending CI: docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04) (waiting on @allada and @MarcusSorealheis)
aaronmondal
assigned adam-singer, allada, MarcusSorealheis and blakehatch and unassigned allada and MarcusSorealheis
aaronmondal
force-pushed
the
rename-cas-to-nativelink
branch
from
874a815
to
c12456b
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 4 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image, ubuntu-20.04, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @adam-singer, @allada, @blakehatch, and @MarcusSorealheis)
Cargo.toml line 19 at r1 (raw file):
Previously, MarcusSorealheis (Marcus Eagan) wrote…
this change got me thinking, does this comment at the top of the file still hold? I feel these may have disappeared.
@allada do you know anything about that?
Done. This was indeed wrong.
There was a problem hiding this comment.
Reviewed 8 of 9 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 1 of 4 LGTMs obtained (waiting on @adam-singer, @blakehatch, and @MarcusSorealheis)
There was a problem hiding this comment.
Dismissed @MarcusSorealheis from a discussion.
Reviewable status:complete! 1 of 1 LGTMs obtained
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:
a discussion (no related file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
Looking into this we'll probably have to make some changes to the existing changelog anyways. What we're currently doing is more or less a copy of the git history. That's explicitly not what we're supposed to do for the changelog. Instead, it should be a short, concise, human-readable thing that only contains relevant information on CVEs, migration paths and major changes.
See: https://github.com/coreinfrastructure/best-practices-badge/blob/main/docs/criteria.md#change-control
TIL on coreinfra github project
This change is