You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@amoro.apache.org by "klion26 (via GitHub)" <gi...@apache.org> on 2024/08/22 03:44:16 UTC
[PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
klion26 opened a new pull request, #3126:
URL: https://github.com/apache/amoro/pull/3126
## Why are the changes needed?
We add conf to fail silent when no git dir before, so that it would not block the
release process, but it may not fail in normal mode also, in this commit
we move the configuration to profile and can be enabled in the release process
Close #315 .
## Brief change log
<!--
Clearly describe the changes made in modules, classes, methods, etc.
-->
-
## How was this patch tested?
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] Run test locally before making a pull request
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on code in PR #3126:
URL: https://github.com/apache/amoro/pull/3126#discussion_r1726361944
##########
pom.xml:
##########
@@ -1324,5 +1324,21 @@
<lucene-dependency-scope>provided</lucene-dependency-scope>
</properties>
</profile>
+ <profile>
+ <id>fail-silent-on-no-git-dir</id>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>pl.project13.maven</groupId>
+ <artifactId>git-commit-id-plugin</artifactId>
+ <version>4.0.0</version>
+ <configuration>
Review Comment:
sorry for not attaching the result tested in my local env, has been attached in the end now
the configuration in the profile will overwrite these in the build configuration. the priority for the configuration is: `configuration in profile(after enabled)` > `build configuration`. can ref the resource[1][2] here.
the result after the profile enabled
the result for default(not enable the profile)
[1] https://maven.apache.org/guides/introduction/introduction-to-profiles.html?spm=5176.28103460.0.0.6077572cRnkc6p#profile-inheritance
[2] https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#Project_Inheritance
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on PR #3126:
URL: https://github.com/apache/amoro/pull/3126#issuecomment-2303618706
@zhoujinsong @zhongqishang Could you please have a look at this when you're free, thanks.
Could you please share where the release script is located in `$AMORO_DIR/tools/releasing/create_binary_release.sh`, if not where is it, I can change it to respect this after this 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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on PR #3126:
URL: https://github.com/apache/amoro/pull/3126#issuecomment-2303623777
> Could you please share where the release script is located in `$AMORO_DIR/tools/releasing/create_binary_release.sh`, if not where is it, I can change it to respect this after this commit.
You can add this new profile here: https://github.com/apache/amoro/blob/master/tools/releasing/create_binary_release.sh#L68
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on PR #3126:
URL: https://github.com/apache/amoro/pull/3126#issuecomment-2331208391
@zhoujinsong @zhongqishang thanks for the review and merging!
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on code in PR #3126:
URL: https://github.com/apache/amoro/pull/3126#discussion_r1726203456
##########
pom.xml:
##########
@@ -1324,5 +1324,21 @@
<lucene-dependency-scope>provided</lucene-dependency-scope>
</properties>
</profile>
+ <profile>
+ <id>fail-silent-on-no-git-dir</id>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>pl.project13.maven</groupId>
+ <artifactId>git-commit-id-plugin</artifactId>
+ <version>4.0.0</version>
+ <configuration>
Review Comment:
I am not sure if the profile plugin configuration will overwrite the default configuration or merge into the default configuration.
As you just added the new configurations, it may seem to be they will be merged into the default configuration, do we have any documentation about profile behavior about 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: commits-unsubscribe@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong commented on PR #3126:
URL: https://github.com/apache/amoro/pull/3126#issuecomment-2325509276
Thanks for the contribution! @klion26
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on PR #3126:
URL: https://github.com/apache/amoro/pull/3126#issuecomment-2306332216
After an offline discussion with @zhongqishang we reached to consense that: add a new profile to enable the `no git dir` check, and make the check disabled default, because this has a better experience with new user which download the code with zip format.
The code has been changed to the above change, please have a look when you're free thanks. @zhoujinsong @zhongqishang
normal build `mvn396 clean package -pl '!amoro-mixed-format/amoro-mixed-format-trino' -DskipTests -Pskip-dashboard-build`
build after the check enabled `mvn396 clean package -pl '!amoro-mixed-format/amoro-mixed-format-trino' -DskipTests -Pskip-dashboard-build -Pfail-on-no-git-dir`
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on PR #3126:
URL: https://github.com/apache/amoro/pull/3126#issuecomment-2324695678
The pr changed to using a profile with configuration inspired by @zhoujinsong
The result can be found below
<img width="1334" alt="WeChatWorkScreenshot_32715e4b-c883-4d0e-822c-dc5e0295fb52" src="https://github.com/user-attachments/assets/d85f879c-2423-484a-9863-fc578ac7ef28">
<img width="1331" alt="WeChatWorkScreenshot_8cf631f0-fd12-46ab-9070-5e0d2bde34ed" src="https://github.com/user-attachments/assets/911ceb28-0afd-40ca-829f-58d147e9d015">
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "zhoujinsong (via GitHub)" <gi...@apache.org>.
zhoujinsong merged PR #3126:
URL: https://github.com/apache/amoro/pull/3126
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AMORO-3152] Make git-commit-plugin fail on no git dir configurable [amoro]
Posted by "klion26 (via GitHub)" <gi...@apache.org>.
klion26 commented on code in PR #3126:
URL: https://github.com/apache/amoro/pull/3126#discussion_r1726361944
##########
pom.xml:
##########
@@ -1324,5 +1324,21 @@
<lucene-dependency-scope>provided</lucene-dependency-scope>
</properties>
</profile>
+ <profile>
+ <id>fail-silent-on-no-git-dir</id>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>pl.project13.maven</groupId>
+ <artifactId>git-commit-id-plugin</artifactId>
+ <version>4.0.0</version>
+ <configuration>
Review Comment:
the configuration in profile will overwrite these in the build configuration. the priority for the configuration is: `configuration in profile(after enabled)` > `build configuration`.
can ref the resource[1][2] here.
[1] https://maven.apache.org/guides/introduction/introduction-to-profiles.html?spm=5176.28103460.0.0.6077572cRnkc6p#profile-inheritance
[2] https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#Project_Inheritance
--
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@amoro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org