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

MODULES-10862 add support for authorized_keys file mode #338

Merged
merged 2 commits into from
Nov 23, 2020

Conversation

simondeziel
Copy link
Contributor

No description provided.

@simondeziel simondeziel requested a review from a team as a code owner November 6, 2020 01:40
@puppet-community-rangefinder
Copy link

accounts::key_management is a type

that may have no external impact to Forge modules.

accounts::user is a type

Breaking changes to this file WILL impact these 3 modules (exact match):
Breaking changes to this file MAY impact these 4 modules (near match):

This module is declared in 3 of 575 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@simondeziel
Copy link
Contributor Author

@david22swan the travis-ci failure are not due to my change so I'd appreciate if you could still take a look ;)

(you've been helpful in the past, hence the direct ping to you, thanks)

@david22swan david22swan self-assigned this Nov 9, 2020
@david22swan
Copy link
Member

@simondeziel Sorry for the wait.
A fix has been put up for the issue you are experiencing and should be merged soon (#339), so if you rebase onto that you should be fine.
In regards to your pr while the change itself look's good I would like it to have some acceptance test's added to it just for peace of mind.

@simondeziel
Copy link
Contributor Author

@simondeziel Sorry for the wait.

That was quite fast actually, so thanks!

A fix has been put up for the issue you are experiencing and should be merged soon (#339), so if you rebase onto that you should be fine.
In regards to your pr while the change itself look's good I would like it to have some acceptance test's added to it just for peace of mind.

I had already added tests that passed ;)

I'll rebase once #339 is merged and hopefully that will trigger another round of tests. Thanks!

Signed-off-by: Simon Deziel <simon@sdeziel.info>
Signed-off-by: Simon Deziel <simon@sdeziel.info>
@simondeziel simondeziel reopened this Nov 9, 2020
@puppet-community-rangefinder
Copy link

accounts::key_management is a type

that may have no external impact to Forge modules.

accounts::user is a type

Breaking changes to this file WILL impact these 3 modules (exact match):
Breaking changes to this file MAY impact these 4 modules (near match):

This module is declared in 3 of 575 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@simondeziel
Copy link
Contributor Author

@david22swan please let me know if I missed anything, thanks!

@david22swan
Copy link
Member

@simondeziel
Apologies for the wait on the reply again.
Anyway took a quick second look and it still look's good to me. Would still like an acceptance test in addition to the unit one you have added but have decided to go ahead and merge.
Thank you for your work and I hope to see more from you in the future 👍

@david22swan david22swan merged commit 2b58494 into puppetlabs:main Nov 23, 2020
@simondeziel
Copy link
Contributor Author

@david22swan thanks! Sorry I got confused by acceptance vs unit tests. I'll keep that in mind for future contributions.

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.

2 participants