Skip to content

Conversation

@ajamato
Copy link

@ajamato ajamato commented Oct 24, 2018

Update .gitignore file to remove IntelliJ generated gradle files


Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

@ajamato ajamato force-pushed the java_sdk_metrics_gitignore branch 2 times, most recently from 78872cf to 3eb2fd5 Compare October 24, 2018 17:30
@ajamato ajamato force-pushed the java_sdk_metrics_gitignore branch from 3eb2fd5 to 279848b Compare October 24, 2018 17:33
@ajamato
Copy link
Author

ajamato commented Oct 24, 2018

@lukecwik

@lukecwik
Copy link
Member

R: @swegner

Intellij provides a button where the "full" version of Gradle can be fetched and used so that you get symbol lookups in Intellij when editing gradle files.

This .gitignore update will prevent "accidental" updates caused by users clicking that button in Intellij but will require devs who are specifically updating Gradle to use something like git add --force <FILE> to ensure it gets added.

The other thing is that our RAT check excludes everything in .gitignore which would mean that we would stop checking gradle-wrapper.properties/gradlew/gradlew.bat for license headers.

Do you think we should go with this or just be diligent during reviews to prevent accidental overwriting of these files?

Also, for users who want to click that button, they could be told to use a local .gitignore (https://medium.com/@peter_graham/how-to-create-a-local-gitignore-1b19f083492b) to avoid the annoying problem of git adding the changes to your commits.

@lukecwik lukecwik requested a review from swegner October 24, 2018 23:00
**/.gradletasknamecache

# Ignore gradle files generated by Intellij
**/gradle-wrapper.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

Does IntelliJ add new wrapper files, or update existing ones? If it's adding new files, it would be nice if we could craft the exclusion here to only apply to the IntelliJ subdirectory and not the actual gradle wrappers.

Copy link
Member

Choose a reason for hiding this comment

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

It updates our existing ones.

@swegner
Copy link
Contributor

swegner commented Oct 24, 2018

I worry that the IntelliJ-based wrapper could have subtle differences that are incompatible with our build, and this will mask the local changes. If I let IntelliJ overwrite the wrapper and then my build starts failing, it will be very difficult to notice that the gradle wrapper is modified.

Is it important that we support this customization? Currently our IntelliJ documentation isn't very good, and it makes it much more difficult to document/support multiple customization. My preference is to pick one supported path and tell everybody to use it.

@ajamato
Copy link
Author

ajamato commented Oct 27, 2018

Closing. Sc

@ajamato ajamato closed this Oct 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants