search

Behat / Gherkin

Gherkin parser, written in PHP for Behat project
MIT License
1.05k stars 92 forks source link

Address review remarks from #129 #163

Closed pfrenssen closed 4 years ago

pfrenssen commented 4 years ago

Ref. https://github.com/Behat/Gherkin/pull/129#discussion_r385131437

ciaranmcnulty commented 4 years ago

These are both valid review comments from @stof, I regret merging so fast since the last commits

Is making this private a BC break now? I guess the project doesn't have a stated BC policy

guilliamxavier commented 4 years ago

@ciaranmcnulty: What about just releasing a v4.6.2 ASAP?

pfrenssen commented 4 years ago

I don't think protected methods are usually considered to be part of a project's API, only the public methods are.

guilliamxavier commented 4 years ago

👍 Moreover this method was only introduced in the patch-version 4.6.1, so I think there is no reason to hold this off

stof commented 4 years ago

@pfrenssen protected methods are covered by backward compatibility, as changing them breaks child classes (and if you don't care about child classes, why using protected rather than private ?)

pfrenssen commented 4 years ago

@stof OK I just opened this because it was your suggestion to change this to private. I don't care about this. Closing.