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: support to set more roles in web.xml #250

Merged
merged 5 commits into from
Jul 11, 2016
Merged

add: support to set more roles in web.xml #250

merged 5 commits into from
Jul 11, 2016

Conversation

zlanyi
Copy link
Contributor

@zlanyi zlanyi commented Jul 5, 2016 •
edited
Loading

Please check the following items before submitting a PR -- thank you!

Note that this project is released with a Contributor Code of Conduct.
By participating in this project you agree to abide by its terms.
Contributor Code of Conduct.

  • There is no existing PR that addresses this problem
  • Mentioned any existing issues in your commit so they get linked and
    closed once this PR gets merged, i.e: Closes #1554 in the body of a commit
  • Followed the instructions in the Contributing document
  • Ran the unit/spec tests and ensured they still pass
  • Added tests to cover the new behaviour
  • Updated the documentation to match the changes
  • When possible, add an entry to the CHANGELOG file
  • Squashed your PR down to a single commit. You may forego this if the PR
    tries to address multiple issues. Though we prefer one PR per feature/fix,
    sometimes that's not feasible. In that case ensure that a single feature/fix
    and associated tests and documentation is bundled up in one commit

Optional, but extra points:

  • Added tests to ensure the old behaviour cannot accidentally be
    reintroduced

@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 5, 2016

Error on Puppet 3 with Ruby 1.9.x:

Gem::InstallError: rest-client requires Ruby version >= 2.0.0.
Using puppetlabs_spec_helper 1.1.1
An error occurred while installing rest-client (2.0.0), and Bundler cannot
continue.
Make sure that gem install rest-client -v '2.0.0' succeeds before bundling.
The command "eval bundle install --without system_tests development --path=${BUNDLE_PATH:-vendor/bundle}" failed 3 times.

@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 11, 2016

Hi,
any news here?:)

Cheers,
Zoltan

@bastelfreak
Copy link
Member

Hi @zlanyi,

thanks for providing the PR: Two little things:

  • Could you please rebase?
  • Please check the used email address in your commits, github is unable to recognize and match them to your account

@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 11, 2016

ah, i've changed my e-mail address, and now :D

@bastelfreak
Copy link
Member

the email address is working now. Please check the git history, there are still merge conflicts.

@zlanyi zlanyi force-pushed the master branch 2 times, most recently from 511cb02 to 78fb042 Compare July 11, 2016 11:07
@bastelfreak
Copy link
Member

travis is green now, but again the email addresses are gone, except for one commit :D. I'm happy to merge this after your fixed the mail addresses.

@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 11, 2016

ah, you are right :D

@bastelfreak
Copy link
Member

could you check why there is a commit from @ltutar? Is this on purpose?

@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 11, 2016 •
edited
Loading

jyaworski merged commit f915c64 into voxpupuli:master 10 days ago
#249

@bastelfreak
Copy link
Member

please rebase and kick it out of your history. You can join us on #voxpupuli on freenode if you need any help for slapping git.

@ltutar
Copy link
Contributor

ltutar commented Jul 11, 2016

I think it is a mistake on my part. I was working on a different PR.

@ltutar
Copy link
Contributor

ltutar commented Jul 11, 2016

Can you just reject my commit for this PR as admin?

@bastelfreak
Copy link
Member

I can't. @zlanyi has to remove it during a rebase

@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 11, 2016

I hope now its finite okey :)

bastelfreak
bastelfreak reviewed Jul 11, 2016
View reviewed changes
manifests/config.pp
@@ -51,6 +51,8 @@
$truststore = $rundeck::truststore,
$truststore_password = $rundeck::truststore_password,
$user = $rundeck::user,
$security_roles_array_enabled = $rundeck::security_roles_array_enabled,
$security_roles_array = $rundeck::security_roles_array,
Copy link
Member

Choose a reason for hiding this comment

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

could you check this please? Maybe you've used hard tabs to align it, please try it with whitespace/soft tabs.

@bastelfreak
Copy link
Member

the history is now looking fine, but the identation got screwed up in a few places. Could you please fix that?

@bastelfreak
Copy link
Member

looks good!

@bastelfreak bastelfreak merged commit c349479 into voxpupuli:master Jul 11, 2016
@zlanyi
Copy link
Contributor Author

zlanyi commented Jul 12, 2016

Hi @bastelfreak
thanks for your support :)

@bastelfreak
Copy link
Member

sure sure, your welcome. I released 3.1.0 yesterday evening which includes your patch.

egeland pushed a commit to fairfaxmedia/puppet-rundeck that referenced this pull request Feb 2, 2017
@zlanyi @egeland
add: support to set more roles in web.xml (voxpupuli#250)
96ae508 
* add: support to set more roles in web.xml

* change: README.md and CHANGELOG.md due to pull request: 250

* kick out the strange commit

* fix: realigment

* remove hard tabs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zlanyi @bastelfreak @ltutar