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

update-build-files: bump buildifier version #21289

Conversation

AlexTereshenkov
Copy link
Member

@AlexTereshenkov AlexTereshenkov commented Aug 10, 2024

Bumping the buildifier version to the latest one available: https://github.com/bazelbuild/buildtools/releases/tag/v7.1.2.

Running this tiny script to produce the versions metadata after downloading the relevant files from the GitHub page:

declare -A platform_mapping
platform_mapping["darwin-arm64"]="macos_arm64"
platform_mapping["darwin-amd64"]="macos_x86_64"
platform_mapping["linux-arm64"]="linux_arm64"
platform_mapping["linux-amd64"]="linux_x86_64"

for f in $(ls buildifier-*); do
    platform=$(echo $f | awk -F- '{print $2"-"$3}') 
    echo "7.1.2|${platform_mapping["$platform"]}|$(shasum -a 256 $f|awk '{print $1}')|$(wc -c $f|awk '{print $1}')"; 
done

@AlexTereshenkov AlexTereshenkov added category:new feature release-notes:not-required PR doesn't require mention in release notes labels Aug 10, 2024
@AlexTereshenkov AlexTereshenkov self-assigned this Aug 10, 2024
@AlexTereshenkov AlexTereshenkov marked this pull request as ready for review August 10, 2024 17:26
@AlexTereshenkov AlexTereshenkov requested a review from huonw August 10, 2024 17:27
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I'm imagining you may've been thinking of this logic with the no-release-notes required and replacement?

  1. Before update-build-files: add support for Buildifier formatter #21208, no-one could be using buildifier
  2. This PR is landing in the same release
  3. Therefore, it's not worth doing either of:
    1. preserving support for version 5.1.0 by just adding 7.1.2 as a new known version, rather than replacing the existing one (since no-one could be actually invoking version 5.1.0)
    2. calling this out in the release notes since no-one will notice the "change" from 5.1.0 to 7.1.2

If so, sound good!

@AlexTereshenkov
Copy link
Member Author

Yes, that's exactly what I thought. I don't think we need to support more than one version since the previous one wasn't usable. The upcoming upgrades will of course respect the most recently added one.

@AlexTereshenkov AlexTereshenkov merged commit 4553e4c into pantsbuild:main Aug 12, 2024
25 checks passed
@AlexTereshenkov AlexTereshenkov deleted the goals/update-build-files/bump-buildifier-version branch August 12, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new feature release-notes:not-required PR doesn't require mention in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants