WIP: Merge __apt_pin fixes in 6.9 to master #336

Closed

fancsali wants to merge 4 commits from 6.9 into master

pull from: 6.9

merge into: ungleich-public:master

ungleich-public:master

ungleich-public:7.0

ungleich-public:py3.10

ungleich-public:ander/__package_apt_update_index

ungleich-public:haproxy-dualstack

ungleich-public:ander/__sed

ungleich-public:beta

ungleich-public:ander/os_version_debian_sid

ungleich-public:evilham-compatibility-fixes

ungleich-public:ander/__rsync

ungleich-public:__snakeoil_cert

ungleich-public:ander/update_readme

ungleich-public:__download_improvements

ungleich-public:feature/onchange

ungleich-public:cleanup/string-formatting

ungleich-public:feature/type-relationship-graph

ungleich-public:cleanup/ssh-auth-keys-types

ungleich-public:__letsencrypt_cert-fix-hooks

ungleich-public:bugfix/preos-debug

ungleich-public:bugfix/in-script-import

ungleich-public:6.8

ungleich-public:bugfix/sphinx-docs-build

ungleich-public:6.7

ungleich-public:cherry-pick-2f433a14

ungleich-public:bugfix/make-code-consistent

ungleich-public:6.6

ungleich-public:regain-py3.2-support

ungleich-public:6.5

ungleich-public:bugfix/multiple-log-lines

ungleich-public:matterbridge

ungleich-public:coturn

ungleich-public:alpinefix

ungleich-public:matrix

ungleich-public:new-type/network-interface

ungleich-public:feature/process

ungleich-public:6.4

ungleich-public:feature/info-command

ungleich-public:feature/libexec

ungleich-public:6.3

ungleich-public:preos-plugins-dir-opt

ungleich-public:gitlab-ci

ungleich-public:6.2

ungleich-public:order-dep-fix

ungleich-public:6.1

ungleich-public:6.0

ungleich-public:build/support-pip-from-git

ungleich-public:feature/shell-lib

ungleich-public:5.1

ungleich-public:feature/support-type-deprecation

ungleich-public:5.0

ungleich-public:feature/python-types

ungleich-public:4.11

ungleich-public:4.10

ungleich-public:shellcheck

ungleich-public:4.9

ungleich-public:4.8

ungleich-public:freebsd-improvements

ungleich-public:new-prometheus

ungleich-public:key_value-onchange

ungleich-public:feature/output_streams

ungleich-public:AnotherKamila-patch-1

ungleich-public:__letsencrypt_cert-fixes

ungleich-public:letsencrypt-cron-fix

ungleich-public:4.7

ungleich-public:newtype-__letsencrypt_cert

ungleich-public:os_explorer_devuan_fix

ungleich-public:prometheus-exporter-fixes

ungleich-public:daemontools-for-fbsd

ungleich-public:type-prometheus-exporter-from-master

ungleich-public:prometheus-more-fixes

ungleich-public:4.6

ungleich-public:4.5

ungleich-public:fix-j

ungleich-public:steven-backport

ungleich-public:4.4

ungleich-public:prometheus-fixes

ungleich-public:grafana_dashboard

ungleich-public:prometheus

ungleich-public:daemontools

ungleich-public:consul_improvements

ungleich-public:feature/trigger

ungleich-public:4.3

ungleich-public:4.0-pre-not-stable

ungleich-public:4.2

ungleich-public:4.1

ungleich-public:4.0

ungleich-public:feature_install_and_preos

ungleich-public:3.1

ungleich-public:no-dot-cdist

ungleich-public:random_dot_cdist

ungleich-public:feature_yum_url

ungleich-public:feature_files_export

ungleich-public:3.0

ungleich-public:2.3

ungleich-public:2.2

ungleich-public:2.1

ungleich-public:ssh_callback

ungleich-public:2.0

ungleich-public:archive_shell_function_approach

ungleich-public:1.7

ungleich-public:1.6

ungleich-public:1.5

ungleich-public:1.4

ungleich-public:1.3

ungleich-public:1.2

ungleich-public:1.1

ungleich-public:1.0

Conversation 7 Commits 4 Files changed 5 +8 -2

fancsali commented 2022-07-01 13:52:36 +00:00

Contributor

Copy link

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

Fix typos; add default priority; comments in generated files 5c96063725

Merge pull request 'Fix typos; add default priority; comments in generated files' (#328) from fancsali/cdist:bugfix/apt-pin-type into 6.9 ... a441a89d97

Reviewed-on: #328

fancsali referenced this pull request 2022-07-01 16:44:27 +00:00

Handle signed-by option in __apt_source #335

nico commented 2022-07-02 17:24:24 +00:00

Owner

Copy link

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?

nico commented 2022-07-02 17:24:52 +00:00

Owner

Copy link

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

fancsali commented 2022-07-08 15:52:46 +00:00

Author

Contributor

Copy link

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

fancsali referenced this pull request 2022-07-08 15:54:56 +00:00

Fix typo in __apt_pin docs #338

nico commented 2022-07-09 12:23:31 +00:00

Owner

Copy link

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

Fix typo in __apt_pin docs d23cef6a1d

Merge pull request 'Fix typo in __apt_pin docs' (#338) from fancsali/cdist:bugfix/apt-pin-type into 6.9 ... e50ea3358a

Reviewed-on: #338

nico referenced this pull request 2022-07-09 12:25:16 +00:00

WIP: Refinements on __apt_source & --signed-by #337

fancsali commented 2022-07-09 13:25:06 +00:00

Author

Contributor

Copy link

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...

fancsali commented 2022-07-14 22:52:06 +00:00

Author

Contributor

Copy link

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.

fnux commented 2024-05-01 11:51:02 +00:00

Collaborator

Copy link

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

Done. Thanks!

> So, if d23cef6a1d can be cherry-picked into master this can also be closed. Done. Thanks!

fnux closed this pull request 2024-05-01 11:51:02 +00:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bugfix

  

cleanup

  

discussion

  

documentation

  

doing

  

done

  

feature

  

improvement

  

packaging

  

Stale

  

testing

  

TODO

No labels

bugfix

cleanup

discussion

documentation

doing

done

feature

improvement

packaging

Stale

testing

TODO

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

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.