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

Avoid expanding mtree spec during analysis phase #576

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

dzbarsky
Copy link
Contributor

@dzbarsky dzbarsky commented Oct 5, 2023

Seeing what CI thinks, I didn't see why we need the empty line at the end of the file

Type of change

  • Bug fix (change which fixes an issue)
  • New feature or functionality (change which adds functionality)
  • Style (white-space, formatting, etc...)
  • Refactor (a code change that neither fixes a bug or adds a new feature)
    - Performance (a code change that improves performance)
  • Documentation (updates to documentation or READMEs)
  • Chore (any other change that doesn't affect source or test files, such as configuration)

For changes visible to end-users

  • Breaking change (this change will force users to change their own code or config)
  • Relevant documentation has been updated
  • Suggested release notes are provided below:

Test plan

- Covered by existing test cases

  • New test cases added
  • Manual testing; please provide instructions so we can reproduce:

content.set_param_file_format("multiline")
content.add_all(ctx.files.srcs, map_each = _default_mtree_line)
# TODO(zbarsky): do we need this blank line? Tests pass on OSX without it for me.
#content.add("")
Copy link
Member

Choose a reason for hiding this comment

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

this was here to ensure the last line of the file has a trailing newline, but I think the multiline param_file_format is doing it so you're right it's unneeded. And yeah I trust our tests pretty well :)

remove comment and unused code
@alexeagle alexeagle merged commit 8fe4f6f into aspect-build:2.x Oct 5, 2023
4 checks passed
@dzbarsky dzbarsky deleted the 2.x branch October 5, 2023 19:37
alexeagle added a commit that referenced this pull request Oct 8, 2023
* Avoid expanding mtree spec during analysis phase

* Update tar.bzl

remove comment and unused code

---------

Co-authored-by: Alex Eagle <alex@aspect.dev>
alexeagle added a commit that referenced this pull request Dec 23, 2023
* Avoid expanding mtree spec during analysis phase

* Update tar.bzl

remove comment and unused code

---------

Co-authored-by: Alex Eagle <alex@aspect.dev>
gregmagolan pushed a commit that referenced this pull request Mar 23, 2024
* Avoid expanding mtree spec during analysis phase

* Update tar.bzl

remove comment and unused code

---------

Co-authored-by: Alex Eagle <alex@aspect.dev>
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.

None yet

2 participants