You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "bereng (via GitHub)" <gi...@apache.org> on 2023/06/13 07:27:00 UTC
[GitHub] [cassandra] bereng opened a new pull request, #2411: Slow builds due to checkstyle
bereng opened a new pull request, #2411:
URL: https://github.com/apache/cassandra/pull/2411
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] Mmuzaf commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228154156
##########
checkstyle.xml:
##########
@@ -23,6 +23,8 @@
<property name="severity" value="error"/>
<property name="fileExtensions" value="java, properties, xml"/>
+
+ <property name="cacheFile" value="${build.dir}/checkstyle_cachefile"/>
Review Comment:
I have checked locally and everything is working as expected using the `cacheFile` property. I'm wondering if it's worth putting the cache file under the `checkstyle` directory `${checkstyle.log.dir}` to keep everything in one place?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on pull request #2411: Slow builds due to checkstyle
Posted by "michaelsembwever (via GitHub)" <gi...@apache.org>.
michaelsembwever commented on PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#issuecomment-1590521371
i know we're editing existing files here, but it really bugs me that the checkstyle xml files are checked in at the top directory.
`checkstyle.xml` `checkstyle_report_test.xml` `checkstyle_test.xml` should be put under `.build/`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228181203
##########
checkstyle.xml:
##########
@@ -23,6 +23,8 @@
<property name="severity" value="error"/>
<property name="fileExtensions" value="java, properties, xml"/>
+
+ <property name="cacheFile" value="${build.dir}/checkstyle_cachefile"/>
Review Comment:
I think you're right. I missed that folder. They belong in there imo and it will still be under build. So all good. I have committed your suggestion. Thx.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "michaelsembwever (via GitHub)" <gi...@apache.org>.
michaelsembwever commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1227804982
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
could we just put these into the build directory instead ? or the user's tmp directory? (i don't know how it works across different clones and repos…?)
(otherwise this will break releases)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] Mmuzaf commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228154156
##########
checkstyle.xml:
##########
@@ -23,6 +23,8 @@
<property name="severity" value="error"/>
<property name="fileExtensions" value="java, properties, xml"/>
+
+ <property name="cacheFile" value="${build.dir}/checkstyle_cachefile"/>
Review Comment:
I have checked locally and everything is working as expected using the `cacheFile` property. I'm wondering if it's worth putting the cache file under the `checkstyle' directory `${checkstyle.log.dir}` to keep everything in one place?
I have checked it locally, but I'm not sure that the ant is resolving the `${build.dir}` placeholder correctly.
##########
checkstyle.xml:
##########
@@ -23,6 +23,8 @@
<property name="severity" value="error"/>
<property name="fileExtensions" value="java, properties, xml"/>
+
+ <property name="cacheFile" value="${build.dir}/checkstyle_cachefile"/>
Review Comment:
I have checked locally and everything is working as expected using the `cacheFile` property. I'm wondering if it's worth putting the cache file under the `checkstyle' directory `${checkstyle.log.dir}` to keep everything in one place?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#issuecomment-1590562528
So you want to move all the checkstyle output from `build/checkstyle` to `.build/checkstyle`? This is moving the whole checkstyle around, are you ok I do this as a separate ticket and get this merged and start enjoying it? :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on pull request #2411: Slow builds due to checkstyle
Posted by "michaelsembwever (via GitHub)" <gi...@apache.org>.
michaelsembwever commented on PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#issuecomment-1590643498
> So you want to move all the checkstyle files from build/checkstyle to .build/checkstyle? This is moving the whole checkstyle around, are you ok I do this as a separate ticket and get this merged and start enjoying it? :-)
No, the versioned controlled checkstyle xml files. (Not talking about the generated files.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng closed pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng closed pull request #2411: Slow builds due to checkstyle
URL: https://github.com/apache/cassandra/pull/2411
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228112153
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
@bereng @michaelsembwever Could we make it a property in build.xml and reference it in checkstyle.xml? That way I can put whatever path I want to `build.properties` file. Once it is in `build` dir, it will be there forever no?
What if I do `ant realclean jar`? Even I have not changed single line of the source code, I will need to do checkstyle again?
People may have a global `.gitignore` in their $HOME (normal git stuff) so I can point my build to cache it somewhere it will be gitignored anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228112153
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
@bereng @michaelsembwever Could we make it a property in build.xml and reference it here? That way I can put whatever path I want to `build.properties` file. Once it is in `build` dir, it will be there forever no?
What if I do `ant realclean jar`? Even I have not changed single line of the source code, I will need to do checkstyle again?
People may have a global `.gitignore` in their $HOME (normal git stuff) so I can point my build to cache it somewhere it will be gitignored anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228145194
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
I don't see a `build.properties` file so I am not following. You mean you want a specific property in build.xml just for the checkstyle cache? Why would sbdy need to change that? :thinking:
You need it cleaned on every 'realclean', if you're checking out branches on the same folder only realclean works and you need to rebuild the checkstyle cache. It's not perfect but it's the best compromise I came up with.
If we put it in the build folder it gets gitignored already. Look at the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] Mmuzaf commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228163023
##########
checkstyle.xml:
##########
@@ -23,6 +23,8 @@
<property name="severity" value="error"/>
<property name="fileExtensions" value="java, properties, xml"/>
+
+ <property name="cacheFile" value="${build.dir}/checkstyle_cachefile"/>
Review Comment:
oh, I'm sorry :-) I have just seen a lot of comments related to this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228112153
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
@bereng @michaelsembwever Could we make it a property in build.xml and reference it here? That way I can put whatever path I want to `build.properties` file. Once it is in `build` dir, it will be there forever no?
What if I do `ant realclean jar`? Even I have not changed single line of the source code, I will need to do checkstyle again?
People may have a global '.gitignore' in their $HOME (normal git stuff) so I can point my build to cache it somewhere it will be gitignored anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228539872
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
@bereng yes, `build.properties` is not there but `build.propeties.default` is. That is supposed to be a template you place your properties and and rename it to `build.properties` which is in our `.gitignore`.
Anyway, I gave it the second thought and I think you all are right. `realclean` should just clean it _all_. We definitely need to have a target which wipes it all out regardless of the properties / settings a developer might set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1227970409
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
Devbranch job seems to have been able to build ok...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "michaelsembwever (via GitHub)" <gi...@apache.org>.
michaelsembwever commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1227804982
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
could we just put these into the build directory instead ? or the user's tmp directory? (i don't know how it works across different clones and repos…?)
(otherwise this will break releases, e.g. deb packaging)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "michaelsembwever (via GitHub)" <gi...@apache.org>.
michaelsembwever commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228135013
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
> Even I have not changed single line of the source code, I will need to do checkstyle again?
You compile classes again, even if you haven't touched them. That's the point with clean, and the behaviour i would expect.
I'm weary of having anything at the top level, it causes problems. Having it under build/ (specifically `${build.dir}`) in in line with the work happening in 18133.
I would also be in favour of not calling checkstyle when jar building. With 18133, all linters are a separate command (e.g. .build/check-code.sh). To me this makes sense, as we also have owasp which is critical but we are ignoring because it's too slow (and network prone).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] Mmuzaf commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228195080
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
My understanding is that the `ant realclean` must clean the checkstyle cache, this is something much closer to the way Maven behaves and much more expected than having a dedicated checkstyle cache somewhere else. I'm +1 here.
I did small local tests by running `ant build` and changing some classes a few times in succession, so that now it doesn't check the files that were not changed. Only the affected files are checked, which in turn speeds up the local build.
```
#1
/Users/x/IdeaProjects/cassandra/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java=1686663255220
#2
/Users/x/IdeaProjects/cassandra/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java=1686663333852
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228112153
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
@bereng @michaelsembwever Could we make it a property? That way I can put whatever path I want to `build.properties` file. Once it is in `build` dir, it will be there forever no?
What if I do `ant realclean jar`? Even I have not changed single line of the source code, I will need to do checkstyle again?
People may have a global '.gitignore' in their $HOME (normal git stuff) so I can point my build to cache it somewhere it will be gitignored anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1227838672
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
I moved it into the build dir. If it were on the tmp folder there could be cross-talk between different builds. Also let's see how this [devbranch](https://ci-cassandra.apache.org/view/all/job/Cassandra-devbranch/2511/) does...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] bereng commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "bereng (via GitHub)" <gi...@apache.org>.
bereng commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1227838672
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
I moved it into the build dir. If it were on the tmp folder there could be cross-talk between different builds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] Mmuzaf commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "Mmuzaf (via GitHub)" <gi...@apache.org>.
Mmuzaf commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228195080
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
My understanding is that the `ant realclean` must clean the checkstyle cache as well, this is something much closer to the way Maven behaves and much more expected than having a dedicated checkstyle cache somewhere else. I'm +1 here.
I did small local tests by running `ant build` and changing some classes a few times sequentially, so that now it doesn't check the files that were not changed. Only the affected files are checked, which in turn speeds up the local build.
```
#1
/Users/x/IdeaProjects/cassandra/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java=1686663255220
#2
/Users/x/IdeaProjects/cassandra/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java=1686663333852
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228539872
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
@bereng yes, `build.properties` is not there but `build.propeties.default` is. That is supposed to be a template you place your properties in and rename it to `build.properties` which is in our `.gitignore`.
Anyway, I gave it the second thought and I think you all are right. `realclean` should just clean it _all_. We definitely need to have a target which wipes it all out regardless of the properties / settings a developer might set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] michaelsembwever commented on a diff in pull request #2411: Slow builds due to checkstyle
Posted by "michaelsembwever (via GitHub)" <gi...@apache.org>.
michaelsembwever commented on code in PR #2411:
URL: https://github.com/apache/cassandra/pull/2411#discussion_r1228135013
##########
.gitignore:
##########
@@ -69,6 +69,8 @@ target/
.DS_Store
Thumbs.db
.ccm/
+checkstyle_cachefile
+checkstyle_test_cachefile
Review Comment:
> Even I have not changed single line of the source code, I will need to do checkstyle again?
You compile classes again, even if you haven't touched them. That's the point with clean, and the behaviour i would expect.
I'm weary of having anything at the top level, it causes problems. Having it under build/ (specifically `${build.dir}`) in in line with the work happening in 18133.
I would also be in favour of not calling checkstyle when jar building. With 18133, all linters are a separate command (e.g. .build/check-code.sh). To me this makes sense, has we also have owasp which is critical but we are ignoring because it's too slow (and network prone).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org