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

Some DocBlocks, comments, formatting because why not try? #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Some DocBlocks, comments, formatting because why not try? #1

wants to merge 3 commits into from

Conversation

marcusmoore
Copy link

No description provided.

@@ -1,6 +1,6 @@
composer.phar
vendor/

.idea*

Choose a reason for hiding this comment

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

Its actually a good idea to put this in your global gitignore, instead of the project one, as not everyone uses phpstorm

Copy link
Member

Choose a reason for hiding this comment

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

What would it matter if someone wasn't using phpStorm? It just wouldn't do anything. What if someone was using phpStorm and didn't have this in their global gitignore? (<= Rhetorical question)

I don't see any harm having it here.

Choose a reason for hiding this comment

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

It adds stuff to the gitignore that isn't necessary.

project gitignores are meant to house things that should be ignored for that specific project

Copy link
Member

Choose a reason for hiding this comment

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

I think you are being overly critical. I will restate my original opinion, I don't see any harm having it here.

Choose a reason for hiding this comment

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

I'm not being critical at all, i'm simply offering an point of view that is commonly accepted by the open source community.

https://github.com/laravel/framework/blob/4.2/.gitignore
https://github.com/symfony/symfony/blob/2.7/.gitignore

just two of the biggest OS php projects out there, as reference.

Copy link
Member

Choose a reason for hiding this comment

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

Noted, thank you

Copy link
Author

Choose a reason for hiding this comment

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

Whoa! I honestly didn't know (or forgot) about a global gitignore file. This is news to me and as Eric said, noted.

I can go either way.

Thanks for the input.

Choose a reason for hiding this comment

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

Theres a really good repo by github, https://github.com/github/gitignore/ that's got a lot of good gitignore information.

They list all of the IDE and system specific ones in the Global directory.

Its a pretty good resource.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we cloned it to the SDPHP Repo sometime ago to bring it to peoples attention

https://github.com/sdphp/gitignore

@cryptiklemur
Copy link

You guys should look at switching to following PSR-2

@marcusmoore
Copy link
Author

A big part of why I threw this out there is to get a discussion going like this.
I'm inclined to believe that this isn't following any sort of standard because it's from a study-group where the participants are at varying levels of skill.
I know that I didn't commit to the PSR-2 standard because I never have before but I'll be looking into it in the next few days.

Thank you again.

@cryptiklemur
Copy link

I'm inclined to believe that this isn't following any sort of standard because it's from a study-group where the participants are at varying levels of skill.

Yeah, good to start the process of following standards easier though.

It wont fit in the scope of this PR to migrate to the PSR-2 standard, but, it would definitely make it easier for newcomers to hop on the project.

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