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

Feature: Keytool #261

Merged
merged 1 commit into from
Sep 8, 2016
Merged

Feature: Keytool #261

merged 1 commit into from
Sep 8, 2016

Conversation

zlanyi
Copy link
Contributor

@zlanyi zlanyi commented Aug 10, 2016

  • feature: generate Keytool files
  • fix: profile.erb due to "RDECK_SSL_OPTS"

@zlanyi
Copy link
Contributor Author

zlanyi commented Aug 10, 2016

Hi,
i did a new pull request because of the previous version of pull request (260) got a very strange commit .

Cheers,
Zoltan

@zlanyi
Copy link
Contributor Author

zlanyi commented Aug 15, 2016

I hope all things okay.

@zlanyi
Copy link
Contributor Author

zlanyi commented Aug 22, 2016

Hi Guys,
any news here?

Cheers,
Zoltan

@jyaworski
Copy link
Member

@zlanyi can you rebase on the current master? It looks good.

@zlanyi
Copy link
Contributor Author

zlanyi commented Sep 1, 2016

"spec/acceptance/rundeck_spec.rb:5:5: C: RSpec/MultipleExpectations: Too many expectations."

@alexjfisher
Copy link
Member

@zlanyi You could probably disable that cop for now by adding

# rubocop:disable RSpec/MultipleExpectations

to the top of the file. eg like in voxpupuli/puppet-archive@fcfc086

Other than than, could you rebase your PR instead of merging from upstream? If you're not that comfortable with git head on over to the #voxpupuli channel on Freenode IRC. Plenty of people willing to help you out there.

Thanks

@zlanyi
Copy link
Contributor Author

zlanyi commented Sep 1, 2016

@jyaworski : i did so the rebase
git checkout master
git pull upstream master
git checkout keytools2
git rebase master

its wrong?

@alexjfisher
Copy link
Member

Ideally, the end result would be one or two logical commits, not the 13 here https://github.com/voxpupuli/puppet-rundeck/pull/261/commits

copy branch to keytool2 because of a strange commit from igalic is my favorite! ;)

Perhaps try a rebase -i master and do some squashing. But maybe best to make sure you have a safe copy of your branch first. git checkout keytools2; git checkout -b keytools2_backup; git checkout keytools2

@@ -504,3 +504,7 @@ RSpec/DescribeClass:
# Example length is not necessarily an indicator of code quality
RSpec/ExampleLength:
Enabled: False

Copy link
Member

Choose a reason for hiding this comment

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

The .rubocop.yml file is managed by modulesync and shouldn't be edited manually. Could you disable the cop in the same manner as in voxpupuli/puppet-archive@fcfc086 by inserting the rubocop control comment at the first line of spec/acceptance/rundeck_spec.rb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@alexjfisher
Copy link
Member

@zlanyi Great! One last thing... You're now back up to 5 commits. Could you squash them down to just a single one with a suitable commit message?

Thanks.

@zlanyi
Copy link
Contributor Author

zlanyi commented Sep 2, 2016

@alexjfisher : thanks your support, done

@alexjfisher
Copy link
Member

@zlanyi I'm really, really sorry for not merging this yet... But... could you fix up the English in the README?
(You can use git commit --amend to avoid creating a new commit)
Thanks.

@@ -65,6 +65,36 @@ Web context path to use, such as "/rundeck". http://host.domain:port/server_web_
#####`ssl_enabled`
Enable ssl for the Rundeck web application.

#####`ssl_keyfile`
#####`ssl_certfile`
If ssl enabled you need set up this. You need provide the crt and key files in your profile oder role manifest.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence needs a little more work in order to be clear to users.

Let's start with something like this:

"If ssl_enabled is True, you must supply this parameter. It is recommended that you provide the .crt and .key files separately via other means, such as a role or profile manifest."

fix: profile.erb due to "RDECK_SSL_OPTS"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants