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

Filtering #26

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Filtering #26

wants to merge 5 commits into from

Conversation

vra5107
Copy link

@vra5107 vra5107 commented Sep 8, 2018

Most of the time resource files for different environments are largely the same. They only have minor differences. In those scenarios it is quite cumborsome to parse through the entire file to find the small changes. It makes sense to put those minor differences in a properties files and move the full files to a common location.

For example take logback.xml file. The file is usually only differs in server address, port and a few other things. It makes sense to move those small differences into a properties file in each of the environment directories and move a single logback.xml file into a common location. So the directory structure looks something like this.

  • src/
    • main/
      • environments/
        • common/
          • logback.xml
        • dev/
          • envspec.properties
        • ci/
          • envspec.properties
        • qa/
          • envspec.properties

Now all wars will have logback.xml and the logback.xml is filtered using envspec.properties for each environment individually.

But multienv-maven currently doesn't support custom filtering for each environment. Infact the readme file mentions this under filtering if I am not wrong.

Different files for different environments

Although AbstractMultiEnvMojo has a field called filters, it isn't being used. This change Makes use of that field and it also introduces another configuration item called commonDir.

With this there will be two steps to filtering:

  1. All files in src/main/environments are filtered using the files listed in the filter tags in build section. This includes the common directory.

  2. Then multienv-maven filters the common files using the key/value pairs from the properties file declared in filters tag inside the configuration segment of multienv maven plugin. If filters are declared but commonDir is not declared an error message will be logged telling the user that the filters declared here are only used to filter files in common directory. It also tells the user if he has no intention of using a common directory he should declare filters in the build section of the pom.

NOTE: This includes the changes from pull request #23

Having a common directory keeps things clean and easy to understand.

@vra5107 vra5107 mentioned this pull request Sep 10, 2018
@khmarbaise
Copy link
Owner

Unfortunately this pull request is introducing two different things one thing filtering which sounds like a good idea and another idea which supports excluding environments which is what I don't want to have cause it's against the base idea having only directories which describes the environments without exception....furthermore it would be great if you could squash your commits...and separate the two things...into two different PR's.

@vra5107
Copy link
Author

vra5107 commented Sep 11, 2018

I was assuming that the pull requests will be approved progressively. If #25 is approved, I can do a merge from master and push. Then #26 will have only the new changes.

I have a legitimate use case for excluding directories in src/main/environments. Consider a situation where there is a need for dev_local, ci_local, qa_local, prod_local etc. When the project is built in Bamboo, it will generate 8 different wars. dev_local, dev, ci_local, ci, qa_local, qa, prod_local, prod. This is confusing and undesirable .

Granted it may appear to be against the base idea. But once a convention to use the src/main/environments to place environment related files is established, it is desirable to stick to that convention. Any new environments should ideally go into src/main/environments. That implies there will be some directories which we may not want processing via multienv-maven-plugin. I will say that the idea of environments covers a larger scope than the plugin and therefore the plugin should reduce hindrances to the larger scope. And I believe in this case lack of this feature is hindrance.

Also, this particular feature doesn't hurt any existing features. It only adds on to them. So I request you to consider it.

This reverts commit 8535e04.

src/main/java/com/soebes/maven/plugins/multienv/ConfigurationMojo.java
src/main/java/com/soebes/maven/plugins/multienv/EnvironmentMojo.java
@vra5107
Copy link
Author

vra5107 commented Sep 12, 2018

Ok, I give up. I have reverted the pull request #25 . Please take a look and accept it if it looks good.

@vra5107 vra5107 mentioned this pull request Sep 12, 2018
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