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

nfpm: Improve clarity of test names and where to packager fields #21310

Merged
merged 5 commits into from
Aug 16, 2024

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Aug 15, 2024

This moves the scripts fields into the <packager>_FIELDS constants to satisfy this review suggestions:
#21299 (comment)
#21310 (comment)

It also adds param.pytest(..., id="...") to name tests in a sane way. I noticed this while splitting up the nfpm PR into multiple PRs, but implementing it across so many PRs would have been painful. So, I saved it for this follow-up PR.

@cognifloyd cognifloyd added category:internal CI, fixes for not-yet-released features, etc. release-notes:not-required PR doesn't require mention in release notes labels Aug 15, 2024
@cognifloyd cognifloyd requested a review from tdyas August 15, 2024 15:44
@cognifloyd cognifloyd self-assigned this Aug 15, 2024
src/python/pants/backend/nfpm/target_types.py Outdated Show resolved Hide resolved
This should make the organization of fields more straightforward.
@cognifloyd cognifloyd changed the title nfpm: Add clarifying comment and test names nfpm: Improve clarity of test names and where to packager fields Aug 16, 2024
@cognifloyd cognifloyd merged commit ba06d69 into main Aug 16, 2024
25 checks passed
@cognifloyd cognifloyd deleted the cognifloyd/nfpm-clarify branch August 16, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc. 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