Magento and grails #968

Closed
wants to merge 10 commits into
from

Projects

None yet

2 participants

@lrkwz
lrkwz commented Feb 27, 2014

New ignore's for magento and grails latest versions

Collaborator
arcresu commented Feb 28, 2014

Thanks for this. In the future, it would be better if you could submit different PRs for different templates.

Note that IDE-specific rules go under the relevant templates in Global/, so for example .iws should only appear in Global/JetBrains.gitignore, and not in the Grails template. Similar for the eclipse-specific rule.

For the Magento template, can you explain why api.php should be ignored by default?

@arcresu arcresu commented on an outdated diff Feb 28, 2014
Magento.gitignore
@@ -37,12 +38,14 @@ app/etc/modules/Phoenix_Moneybookers.xml
app/etc/modules/Cm_RedisSession.xml
app/etc/config.xml
app/etc/enterprise.xml
+app/etc/local.xml
arcresu
arcresu Feb 28, 2014 Collaborator

This rule already exists three lines down.

lrkwz commented Mar 3, 2014

I would ignore api.php as part of the standard magento install as index.php

@arcresu arcresu commented on an outdated diff Mar 6, 2014
Grails.gitignore
@@ -27,4 +28,5 @@
/web-app/plugins
# "temporary" build files
-/target
+
+# other
arcresu
arcresu Mar 6, 2014 Collaborator

This doesn't need to be here.

@arcresu arcresu and 1 other commented on an outdated diff Mar 6, 2014
Grails.gitignore
@@ -27,4 +28,5 @@
/web-app/plugins
# "temporary" build files
-/target
arcresu
arcresu Mar 6, 2014 Collaborator

What will be the impact of removing this rule? Is it something specific to Grails 2.x that should actually still be there for older versions?

lrkwz
lrkwz Mar 25, 2014

Grails will drop the compiled files into /target so IMHO it shouldn't be versioned.

Collaborator
arcresu commented Mar 6, 2014

I've left a couple of inline comments. Also, I'm sorry but I think I've changed my mind: could you please submit these as separate PRs (one per template) after all? The reason is that I'm not convinced that what we're doing with the Magento template is a good idea, and your other changes are independent of this concern.

It will make it a lot easier to advance this if you can explain the need for new rules (e.g. the file is cached data, or contains sensitive information, or something else), as per our contributing guidelines. Thanks!

Collaborator
arcresu commented Mar 6, 2014

See #985 for my reasoning about the Magento template.

Collaborator
arcresu commented Sep 1, 2014

I'll close this since the PR needs to be split up, and it's a bit hard to judge if these changes are a good idea. Feel free to open up new individual PRs if you're still interested in this. Thanks!

@arcresu arcresu closed this Sep 1, 2014
lrkwz commented Sep 1, 2014

ok

-Luca http://about.me/lrkwz

On 1 September 2014 14:11, Carl Suster notifications@github.com wrote:

I'll close this since the PR needs to be split up, and it's a bit hard to
judge if these changes are a good idea. Feel free to open up new individual
PRs if you're still interested in this. Thanks!


Reply to this email directly or view it on GitHub
#968 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment