-
Notifications
You must be signed in to change notification settings - Fork 17
feat(#65): get lints for gh-pages in runtime #137
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
base: master
Are you sure you want to change the base?
Conversation
|
I understand that there are many errors here, but I wanted to do something so that you could specifically point them out to me |
65fd9ec to
db25c7c
Compare
|
@Marat-Tim looks like a good start! Now, please, try to make all build jobs pass. |
63cb8b0 to
525dc81
Compare
|
@yegor256 I'm ready for my code to be destroyed) I have a few questions
|
🚀 Performance AnalysisAll benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information. Click to see the detailed report
|
|
@Marat-Tim the |
|
@h1alexbel please, take a look |
|
@yegor256 We can't replace java code with bash if we want to get a list of motives in java runtime. I also tried to move this executable file to a separate subproject (similar to lints-it), but I ran into the problem that Would it be better to just exclude this file from the build into the final jar? I just don't really understand which plugin this assembly is configured in, if you tell me, it will be very good for me. |
h1alexbel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim check my comments, please
| <version>4.1.1</version> | ||
| <configuration> | ||
| <scripts> | ||
| <script>./.github/motives.groovy</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim let's move the script into src/main/groovy/org/eolang/lints/motives.groovy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@h1alexbel when I tried to do this, I got an error jcabi/jcabi-parent#586
I added a puzzle to this fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@h1alexbel I'm not sure how to check this - could the presence of a groovy folder cause the entire Groovy runtime to be bundled into the final JAR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim in the jcabi-parent, we have this groovy profile:
<profile>
<!--
Compile Groovy sources from src/main/groovy and src/test/groovy
directories.
-->
<id>groovy</id>
<activation>
<file>
<exists>${basedir}/src/main/groovy</exists>
</file>
</activation>
<dependencies>
<dependency>
<groupId>org.codehaus.groovy</groupId>
<artifactId>groovy-all</artifactId>
<version>3.0.24</version>
<type>pom</type>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>org.codehaus.gmavenplus</groupId>
<artifactId>gmavenplus-plugin</artifactId>
<version>4.2.0</version>
<configuration>
<verbose>true</verbose>
</configuration>
<executions>
<execution>
<id>jcabi-groovy</id>
<goals>
<goal>compile</goal>
<goal>compileTests</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@h1alexbel Honestly, I don't quite get it
When I run mvn install without any profiles, I see that classes from the groovy folder end up in target/classes
I'm concerned that this might cause the Groovy runtime to be bundled into the final JAR
Are my concerns unfounded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim correct. Your Groovy scripts should be located in the target/classes. From there you can invoke them using gmavenplus-plugin passing path to the script:
<scripts>
<script>src/main/groovy/org/eolang/lints/SaveMotives.groovy</script>
</scripts>|
|
||
| package org.eolang.lints | ||
|
|
||
| static def saveLint(Lint<?> lint) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim maybe we can setup tests for saveLint() method, in the src/test/groovy? Otherwise, we don't and won't know if we break something during the future modifications. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@h1alexbel when I tried to do this, I got an error jcabi/jcabi-parent#586
I added a puzzle to this fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim maybe you can remove the puzzle, since we released a new version of jcabi-parent?
|
@h1alexbel please check this one |
h1alexbel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim just one comment, please check
.github/motives.groovy
Outdated
| * This script saves a file with the lint name and extension .md to the tmp folder. | ||
| * The motive of this lint is preserved as content. | ||
| * These files are saved for all active lints in the project. | ||
| * @todo #65:30min after fixing this problem with creation of groovy folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim let's make two todos for these issues:
"Move this script to the src/main/groovy folder.""Write a test for motives.groovy"
h1alexbel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim LGTM
|
@yegor256 take a look, please |
yegor256
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a new version of jcabi-parent is available
|
|
||
| package org.eolang.lints | ||
|
|
||
| static def saveLint(Lint<?> lint) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Marat-Tim maybe you can remove the puzzle, since we released a new version of jcabi-parent?
|
@yegor256 Yes, I'm working on it right now - but I've got a question: #137 (comment) |
adds all motives of active lints to the Github pages branch