Skip to content

ARTEMIS-326 Quote logging configuration to account for spaces. Remov…#282

Closed
johnament wants to merge 2 commits intoapache:masterfrom
johnament:ARTEMIS-326
Closed

ARTEMIS-326 Quote logging configuration to account for spaces. Remov…#282
johnament wants to merge 2 commits intoapache:masterfrom
johnament:ARTEMIS-326

Conversation

@johnament
Copy link
Copy Markdown
Contributor

…ed unnecessary duplicate declarations of variable in all poms.

…d unnecessary duplicate declarations of variable in all poms.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

${basedir} represents the directory that this pom is in, absolutely, without any doubts. No reason to redeclare it in every pom.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are some sub projects that will need to find files in reference to the base-dir... (e.g. the native library)... and we had issues with basedir in the past. Maybe it's a versioning thing?

Also this is used on Idea settings. When you run tests. it will pickup the location from this property and it is a bit tricky to get it working. I would need to test and see if it's ok.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. However, when I didn't use the consolidated variable, and instead let it recompute each time, I had to quote on each declaration. It wasn't pretty. I can switch to that if 3.0 compatibility is a must.

@clebertsuconic
Copy link
Copy Markdown
Contributor

checkstyle is not finding the resource.. that variable is not working.

@johnament
Copy link
Copy Markdown
Contributor Author

Lets see if this fixes it. I'm on 3.3.9, so I'm wondering if it may be a difference in variable resolution.

@johnament
Copy link
Copy Markdown
Contributor Author

Ok, it seems like it was a jenkins config issue.

I changed your config to explicitly point to 3.2.5 (which is the minimum version required as per your pom). https://builds.apache.org/job/ActiveMQ-Artemis-PR-Build/jobConfigHistory/

Let me know if you want me to revert it.

Edit:

I just checked w/ infra@a.o. They don't want people using the (latest) tagged mvn versions. That version would have been 3.0.4 which doesn't support the min version tag (and yes, I realize that 3.0.4 is far from the (latest) mvn 3)

@johnament
Copy link
Copy Markdown
Contributor Author

Errr ok. I just looked again and must have been seeing things. Let me know what you'd like to do. 3.0.0 is pretty old as of now, should that really be the min? I can email on the dev list to get more input.

@clebertsuconic
Copy link
Copy Markdown
Contributor

I don't think there should be a problem on bumping the minimal requirement for maven. the only reason nobody did it is because it wasn't needed until now.

@clebertsuconic
Copy link
Copy Markdown
Contributor

These changes are on the right track, however the build is now broken. For example examples are not building, the distribution is not including the examples.. and mvn verify from the examples doesn't work.

@clebertsuconic
Copy link
Copy Markdown
Contributor

On a good note Idea integration still works.. the issue now is on the examples

@johnament
Copy link
Copy Markdown
Contributor Author

I'm going to decline this PR. Looks like my initial issue with single quotes was user error and that's working fine.

@johnament johnament closed this Dec 22, 2015
@johnament johnament deleted the ARTEMIS-326 branch December 22, 2015 01:54
sjhiggs pushed a commit to sjhiggs/activemq-artemis that referenced this pull request Dec 11, 2019
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.

2 participants