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

fix(tar): expose package_dir argument in mtree_mutate #873

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

gzm0
Copy link
Contributor

@gzm0 gzm0 commented Jun 25, 2024

This was likely forgotten in #829 when making the parameters explicit during review.


Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: yes (stardoc)
  • Breaking change (forces users to change their own code or config): no (genrule does not have a package_dir argument)
  • Suggested release notes appear below: no (suggest to use commit message / PR title)

Test plan

  • New test cases added

This was likely forgotten in bazel-contrib#829 when making the parameters explicit
during review.
@gzm0
Copy link
Contributor Author

gzm0 commented Jun 25, 2024

Heads-up: Potentially interacts with #852. /cc @sin-ack

@sin-ack
Copy link
Contributor

sin-ack commented Jun 26, 2024

This shouldn't affect us except for a stray / added in some cases (which does not change the meaning of the mtree spec). Is that an issue? I can move my slash-appender as a separate pass below the package_dir pass if needed.

@gzm0
Copy link
Contributor Author

gzm0 commented Jun 26, 2024

TY for checking. I doubt it's an issue, but I just wanted you to have a look at it, because it seems you have much more mtree spec experience then I do (I don't have any, I just wanted to prepend a string 🤷‍♂️ ).

After re-reading the description on #852, it indeed looks like the two changes should be independent.

@thesayyn thesayyn merged commit 086624a into bazel-contrib:main Jul 2, 2024
24 checks passed
@gzm0 gzm0 deleted the add-package-dir branch July 23, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants