You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "zhfeng (via GitHub)" <gi...@apache.org> on 2023/02/02 09:11:47 UTC
[GitHub] [shardingsphere] zhfeng opened a new pull request, #23937: Upgrade antlr4 to 4.10.1
zhfeng opened a new pull request, #23937:
URL: https://github.com/apache/shardingsphere/pull/23937
Fixes #19990.
Changes proposed in this pull request:
- Bump antlr4 to 4.10.1
- Only run `antlr4-maven-plugin` in JDK 11
- Add generated sources in JDK 8
- After merging this PR, we should clarify on docs and wiki that it needs to use **JDK 11** to build sources at first as it can generate codes for the g4 parseres.
---
Before committing this PR, I'm sure that I have checked the following options:
- [ x My code follows the [code of conduct](https://shardingsphere.apache.org/community/en/involved/conduct/code/) of this project.
- [x] I have self-reviewed the commit code.
- [ ] I have (or in comment I request) added corresponding labels for the pull request.
- [ ] I have passed maven check locally : `./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e`.
- [ ] I have made corresponding changes to the documentation.
- [ ] I have added corresponding unit 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.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415030088
LGTM
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] zhfeng commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415765629
OK, I just wonder why IDEA could find `target\generate-sources\antlr4` as a source directory? It seems that `antlr-maven-plugin` is also running with `generate-sources` phase.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] zhfeng commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415772125
Hmm, it detects `target` and subdirectory automatically.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] TeslaCN commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415757007
> @TeslaCN can you try this setting?
> <img src="https://i.imgur.com/Sp18EiK.png"/>
Of course I can. But I prefer the way that developers could open the project without addtional settings.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] TeslaCN commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415583020
Hi @zhfeng
It seems that you have moved generated directory outside the `target`. The IDEA cannot recognize the code automatically.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] zhfeng commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415730397
@TeslaCN can you try this setting?
<img src="https://i.imgur.com/Sp18EiK.png"/>
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu merged pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] TeslaCN commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "TeslaCN (via GitHub)" <gi...@apache.org>.
TeslaCN commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415671651
> Sorry for the inconvenience. I took these changes because it could build with `JDK 11` to generate the parser codes only once, and `mvn clean install` could work with `JDK 8`. Ony when you change `g4` files, you have to re-generate these codes by `JDK 11`. I suppose that this could less impact on the developers with `JDK 8`.
>
> The for IDEA, I think you have to add source directory manually. such as `src/main/generated`. Do you have any other thought?
I don't think manually add source directory is acceptable. Because we need to add 15 modules manually to start Proxy in IDEA, and these settings might be lost after doing some operation.
I will raise a PR to restore the path of generated codes.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] zhfeng commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1413521400
@TeslaCN @terrymanu can you take a review?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] zhfeng commented on pull request #23937: Upgrade antlr4 to 4.10.1
Posted by "zhfeng (via GitHub)" <gi...@apache.org>.
zhfeng commented on PR #23937:
URL: https://github.com/apache/shardingsphere/pull/23937#issuecomment-1415650731
Sorry for the inconvenience. I took these changes because it could build with `JDK 11` to generate the parser codes only once, and `mvn clean install` could work with `JDK 8`. Ony when you change `g4` files, you have to re-generate these codes by `JDK 11`. I suppose that this could less impact on the developers with `JDK 8`.
The for IDEA, I think you have to add source directory manually. such as `src/main/generated`. Do you have any other thought?
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org