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

Add common lombok config #44

Merged
merged 18 commits into from Jul 12, 2023
Merged

Add common lombok config #44

merged 18 commits into from Jul 12, 2023

Conversation

Tristan-WorkGH
Copy link
Contributor

@Tristan-WorkGH Tristan-WorkGH commented Jul 4, 2023

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?
No.

What kind of change does this PR introduce?
Add a profile (inactive by default) to be use by children projects by activating the correct profile.
It consist of:

  • the common base of lombok configuration in the /lombok.config to be reuse by children
  • a check that the configuration is correctly referenced in the project using it

What is the current behavior?
Each projects have their own lombok.config with duplication and often incomplete.

What is the new behavior (if this is a feature change)?
Each children project can have a common base and update can be centralized.

Does this PR introduce a breaking change or deprecate an API?
No.

Other information:
The idea is to use lombok's import feature to have a common base for GridSuite projects.

Projects that want to use this project will need to:

  • have this module as parent
  • activate this profile: (one of the following)
    • using command line with -Plombok-config-copy and -Plombok-config-check
    • create the file .mvn/lombok-config-copy in the project to activate the copy of the file to be used
    • optionally create the file .mvn/lombok-config-check in the project to activate the check that lombok.config contains the import
  • add this line at the start of /lombok.config
    import target/configs/powsybl-build-tools.jar!powsybl-build-tools/lombok.config

This profiles will, when activate:

  1. during initialize phase: copy the powsybl-build-tools jar in target/configs/ (to be found and use by lombok)
  2. during validate phase: check that the lombok.config file have the correct import at the start of the file

This has been tested with Maven, JetBrains IDEA IntelliJ and Eclipse (with m2e + lombok plugin).

The initialize has been use because of its early position in (Maven lifecycle)[https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html], as IDEs seems to run this phase when reloading/refreshing the project and we need the jar to be present for IDE to parse the configuration (when editing files).

Side note:
No better way found, mainly because of limitation in the way lombok manage internally the import path (ConfigurationFile and FileSystemSourceCache).
And that will not change in near future.

@Tristan-WorkGH Tristan-WorkGH added the enhancement New feature or request label Jul 4, 2023
@Tristan-WorkGH Tristan-WorkGH self-assigned this Jul 4, 2023
powsybl-parent/pom.xml Outdated Show resolved Hide resolved
powsybl-parent/pom.xml Outdated Show resolved Hide resolved
powsybl-parent/pom.xml Outdated Show resolved Hide resolved
@Tristan-WorkGH
Copy link
Contributor Author

Tristan-WorkGH commented Jul 5, 2023

Due to limitation in Maven profiles activation system, like in #2, we use a system of file markers to activate a profile from child projects.

Another note is during my tests, it seems we can't have multiple conditions like <file/> + <property/> or <file><exists/><exist/></file>, as maven only check the first condition...
So the following don't work for activate if a file exist and another not exist at same time: <file><exists>file1</exists><missing>file2</missing></file>

PR description updated

@Tristan-WorkGH
Copy link
Contributor Author

@jonenst I found this other method to copy/extract only the lombok.config from the jar instead of copying the jar lib. A preference?

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-dependency-plugin</artifactId>
    <executions>
        <execution>
            <id>lombok-unpack-config</id>
            <phase>initialize</phase>
            <goals>
                <goal>unpack</goal>
            </goals>
            <configuration>
                <artifactItems>
                    <artifactItem>
                        <groupId>com.powsybl</groupId>
                        <artifactId>powsybl-build-tools</artifactId>
                        <version>${powsybl-build-tools.version}</version>
                        <type>jar</type>
                        <outputDirectory>${project.build.directory}/configs</outputDirectory>
                        <includes>powsybl-build-tools/lombok.config</includes>
                        <overWrite>true</overWrite>
                    </artifactItem>
                </artifactItems>
            </configuration>
        </execution>
    </executions>
</plugin>

powsybl-parent/pom.xml Outdated Show resolved Hide resolved
powsybl-parent/pom.xml Outdated Show resolved Hide resolved
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<showWarnings>true</showWarnings><!--show lombok warning during compile-->
Copy link
Contributor

Choose a reason for hiding this comment

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

separate PR

powsybl-parent/pom.xml Outdated Show resolved Hide resolved
powsybl-parent/pom.xml Outdated Show resolved Hide resolved
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Because of limitation of Maven profiles activation, we decides to switch to files marker instead of property.

Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
Signed-off-by: Tristan Chuine <tristan.chuine_externe@rte-france.com>
This was referenced Jul 13, 2023
@olperr1 olperr1 mentioned this pull request Oct 31, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants