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-5589) - add user/group to war file #253

Closed
wants to merge 5 commits into from
Closed

(MODULES-5589) - add user/group to war file #253

wants to merge 5 commits into from

Conversation

lmayorga1980
Copy link
Contributor

add new parameters to support ownership on the war file.

@lmayorga1980
Copy link
Contributor Author

@adrienthebo , It seems like the README update broke the build but that was just a README change 😕

@pmcmaw pmcmaw requested review from pmcmaw and removed request for pmcmaw September 11, 2017 13:33
README.md Outdated
@@ -1791,6 +1791,15 @@ Default value: `true`.

Valid options: a string containing a `puppet://`, `http(s)://`, or `ftp://` URL.

##### `user`

*The `owner` of the tomcat war file. By default, is set to `tomcat`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to copy a similar format and move the Default value onto a new line.
There are some examples in the README but it would look like something similar to the following:

The 'owner' of the tomcat war file.  

Default value: 'tomcat'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just sent an update and waiting for travis to complete.

README.md Outdated

##### `group`

*The `group` owner of the tomcat war file. By default, is set to `tomcat`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The same as the above comment for 'user'.

@pmcmaw
Copy link
Contributor

pmcmaw commented Sep 11, 2017

@lmayorga1980 I re-kicked Travis, seems it was a transient error and is now passing. 👍

@lmayorga1980
Copy link
Contributor Author

@pmcmaw looks good now?

@pmcmaw
Copy link
Contributor

pmcmaw commented Sep 11, 2017

@lmayorga1980 So the unit tests need updated to include your new parameters.
If you add the new parameters you have implemented into the following test:

    let :params do
      {
        :catalina_base  => '/opt/apache-tomcat/test',
        :app_base       => 'webapps2',
        :war_ensure     => 'present',
        :war_name       => 'sample2.war',
        :war_source     => '/tmp/sample.war',
        :allow_insecure => true,
      }
    end
    it { is_expected.to contain_archive('tomcat::war sample.war').with(
      'source'         => '/tmp/sample.war',
      'path'           => '/opt/apache-tomcat/test/webapps2/sample2.war',
      'allow_insecure' => true,
    )
    }

This test can be found in the following location: 'spec/defines/war_spec.rb'.

@lmayorga1980
Copy link
Contributor Author

lmayorga1980 commented Sep 11, 2017 via email

@pmcmaw pmcmaw changed the title add user/group to war file (MODULES-5589) - add user/group to war file Sep 11, 2017
@lmayorga1980
Copy link
Contributor Author

@pmcmaw added the requested test. 👍

@pmcmaw
Copy link
Contributor

pmcmaw commented Sep 12, 2017

@lmayorga1980 I have created the following PR #254 with your changes and a few minor doc updates. I would like to thank you for your patience and contribution, it is much appreciated. 👍

@pmcmaw pmcmaw closed this Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants