search

zend-patterns / ZendServerSDK

Pure ZF2 CLI for zpk creation and webapi client.
BSD 3-Clause "New" or "Revised" License
22 stars 17 forks source link

Correction for packZpk excluding by default .svn, .git and .cvs patterns #72

Closed marcelotmelo closed 8 years ago

marcelotmelo commented 9 years ago

Hi @slaff, these are the changes I made in order to not include _/ directory and its contents inside the zpk. I really think the documentation section about the default exclusion list should change (https://github.com/zendtech/zendserver-sdk-java/wiki/Deployment-Properties-File#default-exclusion-list). since the tools\conf\excludes.default file no longer exists. Either that or the file should be read and the patterns included as they were on the legacy java version (http://zend-sdk.googlecode.com/svn/trunk/org.zend.sdk.cli/sdkcli/org/zend/sdkcli/internal/mapping/CliMappingLoader.java). I also did minor format corrections and changed a few messages that where not grammatically correct in English. With the changes made a pattern _/something may be included and if there is a "something" directory in the includes tree the directory and its contents will not be packed. The files or directories ending with "something" will be included, though. I also noticed most of the tests from the testGetAbsolute method of the PathInvokableTest class do not pass on Windows, only on Linux (did not test on Mac OS X, but they should pass). I did not correct them since I did not know exactly what should be achieved by the test.

slaff commented 9 years ago
  1. Please, separate different changes in different commits. First make a commit that is changing the coding style, then make a commit with grammar changes and then add commits that change the logic or fix Windows compatibility.
  2. Having hard-coded the default exclusions is not a good idea. It would be better to add the default exclusions in the deployment.properties that is coming when someone calls initZpk. This way the developer has control over the properties without the need to change the source code of the zs-client.
marcelotmelo commented 9 years ago

1 - Ok, I will separate the commits. Can't promise I'll have time to make this today. 2 - The last commit removed the hard coded default exclusions. I agree it is a bad idea. The docs should change, though.

On Tue, Aug 4, 2015 at 6:37 AM, slaff notifications@github.com wrote:

  1. Please, separate different changes in different commits. First make a commit that is changing the coding style, then make a commit with grammar changes and then add commits that change the logic or fix Windows compatibility.
  2. Having hard-coded the default exclusions is not a good idea. It would be better to add the default exclusions in the deployment.properties that is coming when someone calls initZpk. This way the developer has control over the properties without the need to change the source code of the zs-client.

— Reply to this email directly or view it on GitHub https://github.com/zend-patterns/ZendServerSDK/pull/72#issuecomment-127541881 .

marcelotmelo commented 9 years ago

Ok, just to be clear.

Do you want me to make one pull request per commit? Or just separate the commits and make one single pull request is fine?

On Tue, Aug 4, 2015 at 9:32 AM, Marcelo T. de Melo Filho < marcelo.melo.filho@gmail.com> wrote:

1 - Ok, I will separate the commits. Can't promise I'll have time to make this today. 2 - The last commit removed the hard coded default exclusions. I agree it is a bad idea. The docs should change, though.

On Tue, Aug 4, 2015 at 6:37 AM, slaff notifications@github.com wrote:

  1. Please, separate different changes in different commits. First make a commit that is changing the coding style, then make a commit with grammar changes and then add commits that change the logic or fix Windows compatibility.
  2. Having hard-coded the default exclusions is not a good idea. It would be better to add the default exclusions in the deployment.properties that is coming when someone calls initZpk. This way the developer has control over the properties without the need to change the source code of the zs-client.

— Reply to this email directly or view it on GitHub https://github.com/zend-patterns/ZendServerSDK/pull/72#issuecomment-127541881 .

slaff commented 9 years ago

One pull request is enough. Separate the commits as recommended above. In the message for commits that are 100% related to this ticket add sentence like See #71.. One commit per change. For example one commit for all coding-style changes. One commit for all grammar + typos. One commit for all Windows compatibility issues. And so on. You already have the data just need to rebase (and/or squash) the current commits. This way I can more easily cherry-pick plus we will have better history of the discussion(s).

For commits that are related to "Better Windows Compatibility" may be you can create a new issue and associate the commits with that issue.