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.

Sign up for GitHub

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

Jump to bottom

Add options to set custom influxdb repo. #36

Merged
merged 1 commit into from
Sep 13, 2022
Merged

Add options to set custom influxdb repo. #36

merged 1 commit into from
Sep 13, 2022

Conversation

SimonHoenscheid
Copy link
Contributor

@SimonHoenscheid SimonHoenscheid commented Aug 22, 2022 •
edited
Loading

fixes #28

@SimonHoenscheid SimonHoenscheid requested a review from a team as a code owner August 22, 2022 11:40
@puppet-community-rangefinder
Copy link

influxdb is a class

Breaking changes to this file WILL impact these 1 modules (exact match):

This module is declared in 0 of 579 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.

@SimonHoenscheid SimonHoenscheid mentioned this pull request Aug 22, 2022
Closed
m0dular
m0dular suggested changes Aug 22, 2022
View reviewed changes
Copy link
Contributor

@m0dular m0dular left a comment

Choose a reason for hiding this comment

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

Thanks, this is a nice addition 👍

But would it be better to only have the changes from 3c41763 that are related to the repository changes in this branch? We can always rebase the feature branches before merging them, and I think it would play nicer with the changelog generator if each feature only has the related commits.

@SimonHoenscheid
Copy link
Contributor Author

Yes, it was my plan to rebase the commits before these are merged.

@MartyEwings MartyEwings added the enhancement New feature or request label Aug 23, 2022
@SimonHoenscheid
Copy link
Contributor Author

@m0dular It's now a single feature.

m0dular
m0dular approved these changes Aug 24, 2022
View reviewed changes
Copy link
Contributor

@m0dular m0dular left a comment

Choose a reason for hiding this comment

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

👍

@SimonHoenscheid SimonHoenscheid mentioned this pull request Aug 26, 2022
Merged
@SimonHoenscheid
Copy link
Contributor Author

@m0dular added the changes you suggested and rebased. Should be ready to merge.

@SimonHoenscheid
Copy link
Contributor Author

SimonHoenscheid commented Sep 1, 2022 •
edited
Loading

@MartyEwings @m0dular do you need more changes or is the PR ready to merge?

@MartyEwings
Copy link
Contributor

@SimonHoenscheid sorry, there is some leave on my team this week, so I'm doing some prioritisation, ill have a look as soon as i can but it looks good

@m0dular
Copy link
Contributor

m0dular commented Sep 13, 2022

Sorry, I was out the past couple of weeks. This looks good, acceptance tests are green, and I think the existing unit tests cover these changes, so we'll merge it 👍

@m0dular
Add options to set custom influxdb repo
688d7ce 
Prior to this commit, the repository url and gpg key were hard-coded.
This commit adds parameters for customizing these and adds defaults for
EL to the module data.
@m0dular
Copy link
Contributor

m0dular commented Sep 13, 2022

Just a quick note about the git history, I noticed these changes were in a merge commit for the lint changes, #31. I pulled the PR changes out to their own commit, rebased from main to pick up some README additions, and amended the commit so that @SimonHoenscheid is the author. Normally I wouldn't force push someone else's branch, but since this PR is ready I went ahead and did it so we can merge it here.

@m0dular m0dular merged commit 8647446 into puppetlabs:main Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m0dular m0dular m0dular approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

provide alternative path for the repository
3 participants
@SimonHoenscheid @MartyEwings @m0dular