voxpupuli / puppet-jira

Atlassian JIRA Puppet Module
https://forge.puppet.com/puppet/jira
Apache License 2.0
62 stars 143 forks source link

remove incorrect apostrophe in example #332

Closed wolfaba closed 3 years ago

wolfaba commented 3 years ago

this yaml code for jira::java_opts with incorrect apostrophe generates with the template JAVA_OPTS="<%= scope.lookupvar('jira::java_opts') %> $JAVA_OPTS" incorrect code

JAVA_OPTS="-Datlassian.mail.senddisabled=true -Datlassian.mail.fetchdisabled=true -Datlassian.mail.popdisabled=true'
 $JAVA_OPTS"

which generates then error

/opt/jira/atlassian-jira-software-8.13.0-standalone/bin/catalina.sh: eval: line 517: unexpected EOF while looking for matching `''
/opt/jira/atlassian-jira-software-8.13.0-standalone/bin/catalina.sh: eval: line 519: syntax error: unexpected end of file

In fact, without apostrophe, it still generates linebreak in JAVA_OPTS, and then jira does not start. I will create ticket for this issue.

kenyon commented 3 years ago

It would be good if we had a test which fails before making this change.

wolfaba commented 3 years ago

It would be good if we had a test which fails before making this change.

@kenyon is this for me to make some fail-test or is it your internal note? Sorry, I'm not profi-puppet, so I have really no idea what to do :(

kenyon commented 3 years ago

It would be good if we had a test which fails before making this change.

@kenyon is this for me to make some fail-test or is it your internal note? Sorry, I'm not profi-puppet, so I have really no idea what to do :(

@wolfaba this is a note for whoever might want to work on this 😉. To elaborate: if this is really a bug, the tests should have caught it by causing a failure. So if no such failing test exists, that test should be written. Then you can fix the bug and be sure you've fixed it and that it won't reappear without causing a test failure.

danifr commented 3 years ago

I am also affected by this bug. As a workaround I defined all the JAVA_OPTS in a single line

kenyon commented 3 years ago

I just realized that this is just a change to the docs, not the actual code, so disregard my note about tests. Merging. Thanks.