You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/03/25 05:54:58 UTC
[GitHub] [lucene] zacharymorn opened a new pull request #40: LUCENE-9864: Enforce @Override
zacharymorn opened a new pull request #40:
URL: https://github.com/apache/lucene/pull/40
# Description / Solution
Enforce @Override with ecj configuration
# Tests
Passed `./gradlew check`
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/lucene/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Lucene maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `main` branch.
- [x] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807028411
I fixed snowball generator here to add the missing overrides. Here's commit to my local copy that I re-generated the snowball.patch from: https://github.com/rmuir/snowball/commit/fbb3a52c0d4e3130a14ab30496a8caed59fed207
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807558073
> I wondered whether it makes sense to cherry pick those template commits, regenerate, then rebase this patch but maybe it's form over function. In the end it's going to be the same. +1 to commit from me.
I would never do it this way, because I don't think it makes sense to push commits that change generated code for seemingly no reason? You really need the new check, to know that you did it thoroughly and correctly.
Plus: I don't use `git rebase` :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807315632
if i now run `gradlew regenerate` with the branch, there are zero changes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-808497181
I didn't mean to convince you to use it. :) I just mentioned it's not really that scary.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] zacharymorn commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
zacharymorn commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807914012
Thanks everyone for the review and merge! I was wondering last night as well why some code has weird-looking formatting, and should have realized it's generated code. Learn something new everyday!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir merged pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
rmuir merged pull request #40:
URL: https://github.com/apache/lucene/pull/40
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807073242
Oh, we need to fix the `javacc` (or do replacement operations in the gradle task for them) rather than altering the various generated QueryParser code here.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807571035
Thanks. I was too late for review, sorry.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
rmuir commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-808116776
@dweiss as i mentioned, I personally choose not to use it: yes I have tried it, yes I really hate it. Especially rebase -i is really dangerous and bad to me. I think this just depends on the person. It doesn't bother me if someone else wants to rebase things, it really does not impact me. But please understand that it is not for me. I use a merge-based workflow.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807975052
I use rebase all the time on work-in-progress branches. It's really not that scary. rebase -i is particularly useful to do bulk commit edits - it's like batch cherry picking.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
uschindler commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807582673
P.S.: Rebase is a Desaster. I tried it a few times in previous repo. My new repos get pulled and commits stay as they are. No way to modify them by this history changing Rebase operation. Merging and non linear history is real life and better shows what was done and in which order. It also helps to figure out why some problems occurred, because you actually see the merge commit and you can analyze it. Saved my life several times!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #40: LUCENE-9864: Enforce @Override
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #40:
URL: https://github.com/apache/lucene/pull/40#issuecomment-807418821
I wondered whether it makes sense to cherry pick those template commits, regenerate, then rebase this patch but maybe it's form over function. In the end it's going to be the same. +1 to commit from me.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org