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 docs describing behavior of recursive flags #1289

Merged
merged 1 commit into from Mar 12, 2019

Conversation

goosemania
Copy link
Member

@goosemania
Copy link
Member Author

goosemania commented Feb 28, 2019

@dralley , @dkliban , could you review correctness of examples as well, please?

@bherrin3 , let me know if the format of the docs works for you or if you have other suggestions.

I didn't cover at all modular/non-modular RPM difference and repos with mixed content. I'm not sure if it's worth it. Let me know if any of you have preference/ideas what to add.

@goosemania
Copy link
Member Author

@parthaa , your feedback is very welcome as well

@@ -620,11 +620,11 @@ configuration values are optional.
package signature verification.

``recursive`` Boolean flag. If true, units are copied together with their
"latest" dependencies. False by default.
"latest" dependencies. False by default. More information about its usage can be found in :ref:`advanced_copy_between_repositories`.
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be clearer if written as "If true, units are copied together with the latest versions of their dependencies".

@@ -46,7 +46,10 @@ Pulp supports the following modularity_ repository content management use cases:
* publication of the modularity metadata with the repository publication

* copy of one or more modules and/or module defaults between the repositories
* removal of one or more modules and/or module defaults from the repository

* removal of one or more modules and/or module defaults from the repository,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think a semicolon would be better here? Nitpick, no big deal.


| Use case #1: copy module and its artifacts
| and module dependencies
| and atrifacts' *latest* dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, "artifacts"


| Use case #2: copy module and its artifacts
| and module dependencies
| and atrifacts' *missing* dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, "artifacts"

|----baz-1.0.rpm


| Use case #3: copy RPM and its *missing* RPM dependencies
Copy link

@bherrin3 bherrin3 Mar 5, 2019

Choose a reason for hiding this comment

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

Suggested change
| Use case #3: copy RPM and its *missing* RPM dependencies
| Use case #3: copy RPM and its *latest missing* RPM dependencies

Only latest, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, sounds about right

|----bar-0.7.rpm


| Use case #1: copy module and its artifacts and atrifacts' *latest* dependencies
Copy link

Choose a reason for hiding this comment

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

Suggested change
| Use case #1: copy module and its artifacts and atrifacts' *latest* dependencies
| Use case #1: copy module and its artifacts and artifacts' *latest* dependencies

``recursive_conservative`` takes precedence.


Erratum and related RPMs
Copy link

Choose a reason for hiding this comment

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

👍 Thank you for specifically adding this!

Copy link

@bherrin3 bherrin3 left a comment

Choose a reason for hiding this comment

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

@goosemania just adding a few notes.

Talked to the Fedora team about RPMs depending on modular repos/profiles, and while that is possible in the build env, I think it is out of scope for permutation testing.

Thanks for adding the examples and talking with me about the simple case being a real-world case.

Other than editorials, I don't see any logical misses. I will use this as the basis for re-review of the current content coverage.

Thanks!

@goosemania
Copy link
Member Author

@dralley , @bherrin3 , @dkliban , I pushed all the requested changes, ready for re-review/approval

Copy link
Member

@dkliban dkliban left a comment

Choose a reason for hiding this comment

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

Thank you! This looks right to me.

@goosemania goosemania merged commit 5c463a8 into pulp:2-master Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants