WIP: Merge __apt_pin fixes in 6.9 to master #336

Draft
fancsali wants to merge 4 commits from 6.9 into master
Contributor

I might be missing something, but I believe, this has not yet been merged into master, which in turn would mean, this would (potentially) be reverted int he next release...

I might be missing something, but I believe, this has not yet been merged into master, which in turn would mean, this would (potentially) be reverted int he next release...
fancsali added 2 commits 2022-07-01 13:52:37 +00:00
Owner

From the PR it seems that the documenation is not updated - is that intentional?

From the PR it seems that the documenation is not updated - is that intentional?
Owner

You might also rewrite the PR to reflect it's an __apt_pin change instead of a generic message

You might also rewrite the PR to reflect it's an `__apt_pin` change instead of a generic message
fancsali changed title from Make sure 6.9 fixes are in master to Merge __apt_pin fixes to master 2022-07-08 15:45:41 +00:00
fancsali changed title from Merge __apt_pin fixes to master to Merge __apt_pin fixes in 6.9 to master 2022-07-08 15:45:54 +00:00
Author
Contributor

There you go - PR name fixed.

The lack of change in the docs is intentional, the original fix was about documenting the parameter and the default value, but then missing the file(s) to actually make it work as documented.

However, having a look at it, I found a typo; so fixed that and raised #338! ;)

There you go - PR name fixed. The lack of change in the docs is intentional, the original fix was about documenting the parameter and the default value, but then missing the file(s) to actually make it work as documented. However, having a look at it, I found a typo; so fixed that and raised #338! ;)
fancsali changed title from Merge __apt_pin fixes in 6.9 to master to WIP: Merge __apt_pin fixes in 6.9 to master 2022-07-08 15:53:37 +00:00
Owner

If we change the parameter from optional to required, we also need to change the documentation in man.rst with it, otherwise code and documentation mismatch.

Fix required in docs before we can merge this

If we change the parameter from optional to required, we also need to change the documentation in man.rst with it, otherwise code and documentation mismatch. Fix required in docs before we can merge this
nico added 2 commits 2022-07-09 12:23:58 +00:00
Author
Contributor

If we change the parameter from optional to required, we also need to change the documentation in man.rst with it, otherwise code and documentation mismatch.

Fix required in docs before we can merge this

I believe, I didn't explain this PR very well. I do apologise for that.

So, in 6.9, the parameter is optional (defaulting to 500), and is documented as such.

While, in master only the documentation is present in its up-to-date version, the actual code is missing.

That's not really what perspires from the diff, I do understand.

But, here's something I just spotted: it seems, that this change has been already cherry-picked into master, here: https://code.ungleich.ch/ungleich-public/cdist/graph?branch=refs%2Fheads%2Fmaster#bd44c023; so I am rather puzzled, why the PR diff still shows it as it does...

> If we change the parameter from optional to required, we also need to change the documentation in man.rst with it, otherwise code and documentation mismatch. > > Fix required in docs before we can merge this I believe, I didn't explain this PR very well. I do apologise for that. So, in **6.9**, the parameter is optional (defaulting to 500), and is documented as such. While, in **master** only the documentation is present in its up-to-date version, the actual code is missing. That's not really what perspires from the diff, I do understand. But, here's something I just spotted: it seems, that this change has been already cherry-picked into master, here: https://code.ungleich.ch/ungleich-public/cdist/graph?branch=refs%2Fheads%2Fmaster#bd44c023; so I am rather puzzled, why the PR diff still shows it as it does...
Author
Contributor

I did look into this, and found, that Gitea shows the output of

git diff $(git merge-base 6.9 master) 6.9

for this PR; or in other words all the commits/changes that happened on 6.9 since the common ancestor of 6.9 and master.

However, it does not take into account, that 5c96063725 has already been cherry-picked into master as bd44c023d3 a while ago. And I didn't spot that either.

Surely enough, if I do update both master and 6.9 on my end, and merge them, the only thing that actually shows up in the diff for the merge commit is this:

$ git show --first-parent HEAD
commit d07aee03136bf7dd9177bd3c8d9df256ece4777a (HEAD -> master)
Merge: 339ca934 e50ea335
Author: Daniel Fancsali <fancsali@gmail.com>
Date:   Thu Jul 14 23:26:23 2022 +0100

    Trial merge branch '6.9'

diff --git a/cdist/conf/type/__apt_pin/man.rst b/cdist/conf/type/__apt_pin/man.rst
index 4229c0cd..e6ec8b51 100644
--- a/cdist/conf/type/__apt_pin/man.rst
+++ b/cdist/conf/type/__apt_pin/man.rst                                                                                                                        @@ -23,7 +23,7 @@ package
    Package name, glob or regular expression to match (multiple) packages. If not specified `__object_id` is used.

 priority
-   The priority value to assign to matching packages. Deafults to 500. (To match the default target distro's priority)
+   The priority value to assign to matching packages. Defaults to 500. (To match the default target distro's priority)

 state
    Will be passed to underlying `__file` type; see there for valid values and defaults.

Which in turn means, the only difference is the actual fix of spelling of the word 'defaults'.

So, if d23cef6a1d can be cherry-picked into master this can also be closed.

I did look into this, and found, that Gitea shows the output of ``` git diff $(git merge-base 6.9 master) 6.9 ``` for this PR; or in other words all the commits/changes that happened on `6.9` since the common ancestor of `6.9` and `master`. However, it does not take into account, that 5c96063725 has already been cherry-picked into `master` as bd44c023d3 a while ago. And I didn't spot that either. Surely enough, if I do update both `master` and `6.9` on my end, and merge them, the only thing that actually shows up in the diff for the merge commit is this: ``` $ git show --first-parent HEAD commit d07aee03136bf7dd9177bd3c8d9df256ece4777a (HEAD -> master) Merge: 339ca934 e50ea335 Author: Daniel Fancsali <fancsali@gmail.com> Date: Thu Jul 14 23:26:23 2022 +0100 Trial merge branch '6.9' diff --git a/cdist/conf/type/__apt_pin/man.rst b/cdist/conf/type/__apt_pin/man.rst index 4229c0cd..e6ec8b51 100644 --- a/cdist/conf/type/__apt_pin/man.rst +++ b/cdist/conf/type/__apt_pin/man.rst @@ -23,7 +23,7 @@ package Package name, glob or regular expression to match (multiple) packages. If not specified `__object_id` is used. priority - The priority value to assign to matching packages. Deafults to 500. (To match the default target distro's priority) + The priority value to assign to matching packages. Defaults to 500. (To match the default target distro's priority) state Will be passed to underlying `__file` type; see there for valid values and defaults. ``` Which in turn means, the only difference is the actual fix of spelling of the word 'defaults'. So, if d23cef6a1d can be cherry-picked into `master` this can also be closed.
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b 6.9 master
git pull origin 6.9

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff 6.9
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: ungleich-public/cdist#336
No description provided.