pulp / pulp-cli

https://docs.pulpproject.org/pulp_cli/
GNU General Public License v2.0
36 stars 42 forks source link

Add support of contains/startswith to RPM content #933

Closed stanleyz closed 1 month ago

stanleyz commented 6 months ago

Review Checklist:

closes #687

mdellweg commented 6 months ago

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 commented 6 months ago

Thank you for this addition!

stanleyz commented 6 months ago

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 commented 4 months ago

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 commented 4 months ago

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.

stanleyz commented 4 months ago

@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 commented 4 months ago

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

ggainey commented 1 month ago

@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 commented 1 month ago

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

ggainey commented 1 month ago

Closing in preference for #1040