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.

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

Including recommended Symfony gitignore rules #2412

Closed
wants to merge 2 commits into from
Closed

Including recommended Symfony gitignore rules #2412

wants to merge 2 commits into from

Conversation

jezemery
Copy link

@jezemery jezemery commented Jun 9, 2017

Including additional rules recommended by Symfony and basic IDE rule

Reasons for making this change:
To better meet the requirements of Symfony

Links to documentation supporting these rule changes:

ERM?

If this is a new template:

N/A

Including additional rules recommended by Symfony and basic IDE rule
@jezemery
Copy link
Author

jezemery commented Jun 9, 2017

I'd be happy to have feedback if this is rejected

Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

There's a merge conflict due to other PRs being merged. I've left some other comments here about the proposed changes!

@@ -39,3 +42,5 @@

# Backup entities generated with doctrine:generate:entities command
**/Entity/*~
/.idea/
Copy link
Member

Choose a reason for hiding this comment

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

This is a JetBrains rule and more nuanced than ignoring this whole folder - see Global\JetBrains.gitignore

@@ -39,3 +42,5 @@

# Backup entities generated with doctrine:generate:entities command
**/Entity/*~
/.idea/
/vendor/
Copy link
Member

Choose a reason for hiding this comment

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

This is duplicated from line 30

@@ -14,6 +14,8 @@
!var/cache/.gitkeep
!var/logs/.gitkeep
!var/sessions/.gitkeep
!var/SymfonyRequirements.php
/.web-server-pid
Copy link
Member

Choose a reason for hiding this comment

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

This was added in another PR, and can be cleaned up after the merge conflict is resolved.

@shiftkey
Copy link
Member

Closing due to inactivity. @jezemery let me know if there's anything unclear about the feedback I've provided, and if you're able to address those I'm happy to revisit this.

@shiftkey shiftkey closed this Jun 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants