Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#2508] improvement: Remove untracked directories when executing gradle clean #2531

Merged
merged 6 commits into from
Mar 14, 2024
Merged

[#2508] improvement: Remove untracked directories when executing gradle clean #2531

merged 6 commits into from
Mar 14, 2024

Conversation

yijhenlin
Copy link
Contributor

@yijhenlin yijhenlin commented Mar 14, 2024 •
edited
Loading

What changes were proposed in this pull request?

Remove directory spark-warehouse when exectue gradle clean in moudle catalog-lakehouse-iceberg.
Remove directory src/main/resources when execute gradle clean in module server, since which is generated by gradle build.

Why are the changes needed?

Fix: #2508

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

./gradlew :catalogs:catalog-lakehouse-iceberg:clean
./gradlew :server:clean

@mchades
Copy link
Contributor

mchades commented Mar 14, 2024 •
edited
Loading

@yuqi1129 Should we also add spark-warehouse to the .gitignore?

@mchades mchades requested a review from yuqi1129 March 14, 2024 07:55
@yuqi1129
Copy link
Contributor

@mchades
I think so, #2529 has done similar work for module integration-test.

@yijhenlin
Copy link
Contributor Author

yijhenlin commented Mar 14, 2024 •
edited
Loading

@yuqi1129
I add spark-warehouse to .gitignore.
And another question, there is an untracked empty directory shown by git clean -d -n after execute gradle clean

$ git clean -d -n
Would remove server/src/main/resources/

In .gitignore there exist server/src/main/resources/project.properties but gradle server:clean remove the file and leave the empty directory server/src/main/resources
Do I need to deal with this one? if so, what is your expectation?

  1. change server/src/main/resources/project.properties to server/src/main/resources in .gitignore
  2. remove server/src/main/resources when execute gradle server:clean

@yuqi1129
Copy link
Contributor

File server/src/main/resources/project.properties is auto-generated by gradle build, we can remove it when executing gradle clean

@yijhenlin

@yijhenlin yijhenlin changed the title [#2508] improvement(catalog-lakehouse-iceberg): Remove directory spark-warehouse when exectue gradle clean [#2508] improvement: Remove untracked directories after executing gradle clean Mar 14, 2024
@yijhenlin yijhenlin changed the title [#2508] improvement: Remove untracked directories after executing gradle clean [#2508] improvement: Remove untracked directories when executing gradle clean Mar 14, 2024
yuqi1129
yuqi1129 reviewed Mar 14, 2024
View reviewed changes
server/build.gradle.kts
@@ -114,6 +114,6 @@ tasks {
environment("GRAVITINO_TEST", "true")
}
clean {
delete("$propertiesFile")
delete(file(propertiesFile).getParentFile())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, are you going to remove the directory resources? I don't believe we need to take any action here. as
line

delete("$propertiesFile")

has done the work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the directory is generated by gradle build
https://github.com/datastrato/gravitino/blob/83e0e6b7a0b9dea689c90e34416a195385bbe835/server/build.gradle.kts#L87
currently, gradle clean remove the file inside and leave the empty directory, which is untracked by git.

yuqi1129
yuqi1129 approved these changes Mar 14, 2024
View reviewed changes
Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yuqi1129 yuqi1129 merged commit e65ba06 into apache:main Mar 14, 2024
12 checks passed
@yuqi1129
Copy link
Contributor

@yijhenlin
Thank you for your contributions.

@yijhenlin yijhenlin deleted the issue_2508 branch March 15, 2024 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuqi1129 yuqi1129 yuqi1129 approved these changes

Assignees

@yijhenlin yijhenlin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Improvement] Remove directory spark-warehouse when exectue gradle clean in moudle catalog-lakehouse-iceberg
3 participants
@yijhenlin @mchades @yuqi1129