feat(#909): move to class; read from resources #943
Conversation
There was a problem hiding this comment.
@l3r8yJ Thank you for the contribution! I like the idea with the License class. Let's keep it. Moreover, you right that we need to read LICENSE.txt from resources. However, there is one significant issue with the solution you sent: we still need to keep 2 license files in the project root and in the src/main/resources. The original issue we aimed to solve were to remove this redundancy. What do you think if we will copy LICENSE.txt from the root directory to src/main/resources or to ${project.build.directory}/generated-sources as a part of Maven build process? By doing this we will be able to leave only one LICENSE.txt.
|
@volodya-lombrozo when I first started, I considered this approach, but at the time, it felt a bit overly complicated for this type of problem. However, I now think it makes sense, and I’ll implement it using a single file |
There was a problem hiding this comment.
@l3r8yJ Most probably, you can just use the following Maven configuration:
<plugin>
<artifactId>maven-resources-plugin</artifactId>
<version>3.3.1</version>
<executions>
<execution>
<id>copy-resources</id>
<!-- here the phase you need -->
<phase>compile</phase>
<goals>
<goal>copy-resources</goal>
</goals>
<configuration>
<outputDirectory>${project.build.outputDirectory}</outputDirectory>
<resources>
<resource>
<directory>${project.basedir}</directory>
<includes>
<include>LICENSE.txt</include>
</includes>
</resource>
</resources>
</configuration>
</execution>
</executions>
</plugin>However, I haven't test it yet.
|
@volodya-lombrozo oh, thanks, I'll try it |
|
@volodya-lombrozo it worked, take a look, please |
|
|
||
| @Override | ||
| public String value() { | ||
| return new TextOf(new ResourceOf(this.name)).toString(); |
There was a problem hiding this comment.
@l3r8yJ There is a potential performance issue related to this line. We will definitely use this method thousands of times. As a result, each time, we will be required to read the license from the resources, which will significantly slow down the plugin. I would suggest adding one more puzzle for this here. What do you think?
There was a problem hiding this comment.
@volodya-lombrozo I think we can add some cache here
There was a problem hiding this comment.
@l3r8yJ Yes, something like Sticky object from cactoos.
There was a problem hiding this comment.
@l3r8yJ Seems, there are some qulice complaints. Could you fix them please?
|
@volodya-lombrozo can you restart CI? I've checked another time locally |
@l3r8yJ Shure, I've restarted them. Have you tried to run the same command using Java 8? Maybe it's something specific to the Java version. |
|
@volodya-lombrozo can you check it with Java 8 on your environment? I'm getting an infinite build. Also, we can temporary turn off Java 8 on CI and add the puzzle to return it back, wdyt? |
@l3r8yJ It would be the best option. Would you be able to disable it in this PR then? |
|
@volodya-lombrozo sure |
|
@volodya-lombrozo added a puzzle, take a look, please |
There was a problem hiding this comment.
@l3r8yJ Seems, there is one more problem with pipelines. Some of them are failed.
Closes: #909
@volodya-lombrozo take a look, please