You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "dk2k (via GitHub)" <gi...@apache.org> on 2023/03/29 14:14:59 UTC
[GitHub] [camel] dk2k opened a new pull request, #9694: Dk2k code inspection
dk2k opened a new pull request, #9694:
URL: https://github.com/apache/camel/pull/9694
# Description
Fixes for code issues found by inspection.
# Target
main branch
# Tracking
https://issues.apache.org/jira/browse/CAMEL-19217
# Apache Camel coding standards and style
- [x] I checked that each commit in the pull request has a meaningful subject line and body.
- [ x] I formatted the code using `mvn -Pformat,fastinstall install && mvn -Psourcecheck`
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] dk2k commented on pull request #9694: Dk2k code inspection
Posted by "dk2k (via GitHub)" <gi...@apache.org>.
dk2k commented on PR #9694:
URL: https://github.com/apache/camel/pull/9694#issuecomment-1488924011
In the ideal world this cherry pick approach would work, but not in this case. I fixed checkstyle issues (I doubt that some of them caused by my changes) in the end - there were several commits.
For example, automated change from my side:
(b << 0) become (b)
and only in the end I removed redundant parenthesis. Another example: made a field static final. In the end I had to rename it and reorder the declaration. The fixes for checkstyle got mixed up. The PR is consistent, but it's eventual consistency.
Please consider the option of reviewing separate commits from this PR. These commits can be distributed between several reviewers or you can decide for yourself: today I review this commit, tomorrow I will review that one.
For me the reworking will take too much additional time. I will have to build each separate branch. Easy and brute solution is to take a new branch and say copy camel-core folder with the changes. But the readibility will suffer: all the inspection types will get mixed up.
As for building. In ths page https://camel.apache.org/manual/building.html it states:
A normal build without running tests but auto formatting and checkstyle verification enabled
mvn clean install -Pfastinstall,format,sourcecheck
It's fair if the build is OK right from the first attempt. But if some attempt fails, you should execute it without clean target:
mvn install -Pfastinstall,format,sourcecheck
It will rebuild only what needs rebuilt and saves time.
I won't be able to provide digital data to justify optimizations.
For now I will leave the PR open, maybe you will change your mind.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] dk2k commented on pull request #9694: Dk2k code inspection
Posted by "dk2k (via GitHub)" <gi...@apache.org>.
dk2k commented on PR #9694:
URL: https://github.com/apache/camel/pull/9694#issuecomment-1488768114
Is it possible to break it after it was submitted into my forked repository? I thought separating changes into commits per issue type would make reviewing more comfortable
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on pull request #9694: Dk2k code inspection
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9694:
URL: https://github.com/apache/camel/pull/9694#issuecomment-1488940177
I am sorry, but -1 for this change is final. As it is, it won't even be automatically tested by our GitHub automation (as there are too many affected components).
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] dk2k closed pull request #9694: Dk2k code inspection
Posted by "dk2k (via GitHub)" <gi...@apache.org>.
dk2k closed pull request #9694: Dk2k code inspection
URL: https://github.com/apache/camel/pull/9694
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] github-actions[bot] commented on pull request #9694: Dk2k code inspection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9694:
URL: https://github.com/apache/camel/pull/9694#issuecomment-1488712618
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:warning: Please note that the changes on this PR may be **tested automatically**.
If necessary Apache Camel Committers may access logs and test results in the job summaries!
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel] orpiske commented on pull request #9694: Dk2k code inspection
Posted by "orpiske (via GitHub)" <gi...@apache.org>.
orpiske commented on PR #9694:
URL: https://github.com/apache/camel/pull/9694#issuecomment-1488787005
> Is it possible to break it after it was submitted into my forked repository? I thought separating changes into commits per issue type would make reviewing more comfortable
Yes. Follow these steps:
1. Create a branch that groups certain types of fixes logically (i.e.: one branch for fixing logging issues, another for fixing Collection issues, another for fixing cast/conversion issues, and so on).
2. Cherry pick the changes accordingly on each of those branches (please do make sure to test them)
3. Review the commit messages to ensure they follow the description patterns we use in the project (i.e.: have a ticket on them or use `"(chores) name-of-the-component: commit message")
4. Make sure that even so the PR is not too large (otherwise break it even further)
5. Send one PR for each branch
Additional tips:
- be very conservative when changing `core` code (ideally, separate the PRs for `core` changes and `component` changes)
- if there's a performance benefit of the change you are making, please make sure to contribute some numbers, reasoning or provide JMH tests we can use to verify (specially for larger changes on the core code).
This is +/- the process I follow myself.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org