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

Add support of contains/startswith to RPM content #933

Closed
wants to merge 1 commit into from

Conversation

stanleyz
Copy link
Contributor

@stanleyz stanleyz commented Mar 23, 2024

Review Checklist:

  • An issue is properly linked. [feature and bugfix only]
  • Tests are present or not feasible.
  • Commits are split in a logical way (not historically).

closes #687

@mdellweg
Copy link
Member

Please squash the commits and add fixes ... to the commit message.
Also I'm pretty sure that we need some version guards for these options (See "versions dependent codepaths" in the docs).

@mdellweg
Copy link
Member

Thank you for this addition!

@stanleyz
Copy link
Contributor Author

stanleyz commented Mar 24, 2024

Please squash the commits and add fixes ... to the commit message. Also I'm pretty sure that we need some version guards for these options (See "versions dependent codepaths" in the docs).

Thanks @mdellweg for the review, please check again.

@mdellweg
Copy link
Member

One last request.
Please add a file CHANGES/687.feature that contains a one-line changelog. A single past tense sentence. Maybe:
"Added --arch-contains, ... filters to pulp rpm content list."

... and make black should help to get the formatter happy.

@stanleyz
Copy link
Contributor Author

One last request. Please add a file CHANGES/687.feature that contains a one-line changelog. A single past tense sentence. Maybe: "Added --arch-contains, ... filters to pulp rpm content list."

... and make black should help to get the formatter happy.

check again please thanks.

@mdellweg mdellweg enabled auto-merge (rebase) May 21, 2024 09:21
mdellweg
mdellweg previously approved these changes May 21, 2024
auto-merge was automatically disabled May 22, 2024 07:14

Head branch was pushed to by a user without write access

@stanleyz
Copy link
Contributor Author

@mdellweg pushed one additional change to move my test up a bit so that it doesn't interfere with the test OUTPUT in line 132 of file tests/scripts/pulp_rpm/test_content.sh

@mdellweg
Copy link
Member

Can you add fixes #687 to the commit message again.
Also did you manage to run the tests locally?

@ggainey
Copy link
Contributor

ggainey commented Aug 14, 2024

@stanleyz Sorry for the really long pause here, we lost sight of your contribution. I hope you don't mind, in the process of checking the failures I realized what the problem was, and submitted a fixed PR - #1040 starting from your commit.

If you're ok with this, I'd like to close this PR and use #1040 to get this added. Let me know!

@stanleyz
Copy link
Contributor Author

Thanks @ggainey, feel free to go ahead, busy in other things, didn't look too much into this.

@ggainey
Copy link
Contributor

ggainey commented Aug 15, 2024

Closing in preference for #1040

@ggainey ggainey closed this Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for new rpm-content-package filters
3 participants