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

Change properties and methods name to follow PSR standards #128

Closed
wants to merge 3 commits into from

Conversation

ivanlanin
Copy link
Contributor

Changes:

  • Remove underscore from name (on Writer folder)
  • Use camel case name (on some legacy methods)
  • Put @codeCoverageIgnore in HashTable and Shared/ZipStreamWrapper

@ivanlanin
Copy link
Contributor Author

I've passed unit tests and ran all samples.

@Progi1984
Copy link
Member

@gabrielbull What do you think about that ? Is that we should not expect support for namespaces #58 ? I think that permit to change once users' code.

@gabrielbull
Copy link
Member

Changing method names is a major release because it breaks compatibility so it should be 0.8.0. But, unfortunately, I don't think we'll have namespaces so soon. So maybe we should consider releasing the develop branch under 0.8.0 and start implementing namespace for 0.9.0.

@ivanlanin
Copy link
Contributor Author

The names that I changed are mainly privates. The public method names that I changed have very limited use or not used at all. But, I'm ok with any decision :) Please advise.

@gabrielbull
Copy link
Member

Well there is no downsides to have many release so I'd stick with a major version before namespacing.

@Progi1984
Copy link
Member

@gabrielbull So you think that the next version after 0.7.1 should be 0.8.0 if we implement namespacing ?

@gabrielbull
Copy link
Member

I think every version that breaks compatibility should be a 0.x version. Because people are doing this in their composer.json: "phpoffice/phpword": "0.7.*" expecting every minor release (0.7.x) to be bug fixes and such and not break compatibility.

So with that in mind, I think the next release should be 0.8.0 rather than 0.7.1 because we made a TON of changes and maybe there is new bugs that we didn't catch that may break compatibility with projects using 0.7.*.

From there, we continue working on patching things and adding features to the 0.8.* version and start implementing namespaces for the 0.9.0 release.

@ivanlanin
Copy link
Contributor Author

I'll follow whatever the decision :) I'm currently working on #18 issue using my branch. I'm ready to request a pull. Should I abort this pull requests and sync back with develop?

@Progi1984
Copy link
Member

@gabrielbull I agree with you. Thanks for sharing your experience.

So the next version will be 0.8.0 (actually named 0.7.1).

And the next development version will be 0.9.0 - actually named 0.7.2 -, with namespacing and the @ivanlanin's work.

@Progi1984 Progi1984 added this to the 0.7.2 milestone Mar 14, 2014
@ivanlanin
Copy link
Contributor Author

Ok. I withdraw this pull request, keep the naming-std branch to be merged later, and fallback to the current develop track for now.

@ivanlanin ivanlanin closed this Mar 14, 2014
@ivanlanin ivanlanin deleted the naming-std branch March 23, 2014 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants