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

Refs #23204 - Consistent puppet environment naming in hammer #629

Merged
merged 4 commits into from Jul 3, 2019
Merged

Refs #23204 - Consistent puppet environment naming in hammer #629

merged 4 commits into from Jul 3, 2019

Conversation

ofedoren
Copy link
Contributor

Moved the puppet-environment name resolution to hammer-cli-foreman, see: theforeman/hammer-cli-foreman#401

@theforeman-bot
Copy link

@ofedoren, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link

Issues: #23204

@ofedoren ofedoren mentioned this pull request Feb 12, 2019
Merged
akofink
akofink suggested changes Feb 13, 2019
View reviewed changes
Copy link
Member

@akofink akofink left a comment

Choose a reason for hiding this comment

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

You need to also remove the require.

@theforeman-bot
Copy link

@ofedoren, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

@ofedoren
Copy link
Contributor Author

@akofink, right, thanks! Updated.

akofink
akofink approved these changes Feb 14, 2019
View reviewed changes
Copy link
Member

@akofink akofink left a comment

Choose a reason for hiding this comment

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

ACK! Thanks @ofedoren!

@akofink
Copy link
Member

akofink commented Feb 21, 2019

Please ping me when theforeman/hammer-cli-foreman#401 is merged. Thanks!

@ofedoren ofedoren force-pushed the bug-23204-puppet-environment branch from 9ff9fe9 to 7340ca9 Compare April 23, 2019 17:36
@theforeman-bot
Copy link

@ofedoren, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

@ofedoren ofedoren force-pushed the bug-23204-puppet-environment branch from 7340ca9 to a23a231 Compare April 23, 2019 17:41
@ofedoren
Copy link
Contributor Author

ofedoren commented Apr 23, 2019 •
edited

@akofink, there are some extra changes. Same logic as in theforeman/hammer-cli-foreman#401: changes --environment options to --lifecycle-environment.

Also, tests will be red without testing it with original PR (locally they pass).

akofink
akofink approved these changes Apr 25, 2019
View reviewed changes
Copy link
Member

@akofink akofink left a comment

Choose a reason for hiding this comment

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

Great work, @ofedoren! ACK!

@akofink
Copy link
Member

akofink commented Apr 25, 2019

Please ping me when the hammer-cli-foreman PR merges so we can merge this.

@akofink
Copy link
Member

akofink commented Apr 25, 2019

Also, you may want to make a community post about this change. It may break some people's hammer scripts, including our bats tests.

@ofedoren ofedoren force-pushed the bug-23204-puppet-environment branch from a23a231 to d059ffb Compare May 24, 2019 13:55
@ofedoren ofedoren force-pushed the bug-23204-puppet-environment branch from 27de204 to 7dca6f4 Compare May 26, 2019 19:23
@akofink
Copy link
Member

akofink commented May 28, 2019

@ofedoren You may change the option expansion code, but know that content view version IDs are a bit tricky to resolve - you need the content view ID AND either the version (i.e. 3.5) OR the lifecycle environment (i.e. Development). I don't believe there is a test currently for delete with ID resolution (or at all, for that matter :/ )

@ofedoren
Copy link
Contributor Author

ofedoren commented Jul 2, 2019

@akofink, the hammer-cli-foreman PR is merged.

@Katello Katello deleted a comment from theforeman-bot Jul 2, 2019
@Katello Katello deleted a comment from theforeman-bot Jul 2, 2019
@Katello Katello deleted a comment from theforeman-bot Jul 2, 2019
@Katello Katello deleted a comment from theforeman-bot Jul 2, 2019
@chris1984
Copy link
Member

rekicked the tests

@theforeman-bot
Copy link

@ofedoren, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • f954bf7 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.

This message was auto-generated by Foreman's prprocessor

@ofedoren ofedoren force-pushed the bug-23204-puppet-environment branch from f954bf7 to 0ece526 Compare July 3, 2019 11:54
@theforeman-bot
Copy link

@ofedoren, the Redmine ticket used is for a different project than the one associated with this GitHub repository. Please either:

If changing the ticket number used, remember to update the PR title and the commit message (using git commit --amend).

This message was auto-generated by Foreman's prprocessor

akofink
akofink approved these changes Jul 3, 2019
View reviewed changes
Copy link
Member

@akofink akofink left a comment

Choose a reason for hiding this comment

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

Still works as expected. ACK Nice work @ofedoren!

@akofink akofink merged commit d70bd7a into Katello:master Jul 3, 2019
@ekohl
Copy link
Member

ekohl commented Jul 4, 2019

Looks like tests started to fail after this: https://ci.theforeman.org/job/hammer-cli-katello-master-source-release/10/

@akofink
Copy link
Member

akofink commented Jul 8, 2019 •
edited

Yes, you cannot successfully run the hammer-cli-katello master branch against the old released version of hammer-cli-foreman. If you run everything from master, it works fine. We will need a hammer-cli-foreman and hammer-cli release.

ofedoren added a commit to ofedoren/hammer-cli-katello that referenced this pull request Jul 25, 2019
@ofedoren
Refs #23204 - Consistent puppet environment naming in hammer (Katello…
e3feef7 
…#629)

* Refs #23204 - Consistent puppet environment naming in hammer

* Refs #23204 - Clean up --env options

* Refs #23204 - Support --environment-ids

* Refs #23204 - Fixed rubocop
@ofedoren ofedoren mentioned this pull request Jul 25, 2019
Merged
akofink pushed a commit that referenced this pull request Jul 25, 2019
@ofedoren @akofink
Refs #23204 - Consistent puppet environment naming in hammer (#629) (#…
d94b4c8 
…667)

* Refs #23204 - Consistent puppet environment naming in hammer

* Refs #23204 - Clean up --env options

* Refs #23204 - Support --environment-ids

* Refs #23204 - Fixed rubocop
jjeffers pushed a commit to jjeffers/hammer-cli-katello that referenced this pull request Aug 7, 2019
@ofedoren
Refs #23204 - Consistent puppet environment naming in hammer (Katello…
22916cd 
…#629) (Katello#667)

* Refs #23204 - Consistent puppet environment naming in hammer

* Refs #23204 - Clean up --env options

* Refs #23204 - Support --environment-ids

* Refs #23204 - Fixed rubocop
Conflicts:
	lib/hammer_cli_katello/content_view_version.rb
jjeffers pushed a commit to jjeffers/hammer-cli-katello that referenced this pull request Aug 26, 2019
@ofedoren
Refs #23204 - Consistent puppet environment naming in hammer (Katello…
5d63ddb 
…#629) (Katello#667)

* Refs #23204 - Consistent puppet environment naming in hammer

* Refs #23204 - Clean up --env options

* Refs #23204 - Support --environment-ids

* Refs #23204 - Fixed rubocop
(cherry picked from commit d94b4c8)

Conflicts:
	lib/hammer_cli_katello/content_view_version.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akofink akofink akofink approved these changes

Assignees
No one assigned
Labels
Needs testing Not yet reviewed Requires Hammer PR
Projects
None yet
Milestone
No milestone
5 participants
@ofedoren @theforeman-bot @akofink @chris1984 @ekohl