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

Apply Doctrine Coding Standard #77

Merged
merged 26 commits into from
Jun 11, 2018
Merged

Apply Doctrine Coding Standard #77

merged 26 commits into from
Jun 11, 2018

Conversation

VolCh
Copy link
Contributor

@VolCh VolCh commented May 10, 2018

Renamed in /src:

\GeneratedHydratorBenchmark\{HydrationBenchInterface -> HydrationBench}
\GeneratedHydrator\ClassGenerator\{HydratorGeneratorInterface -> HydratorGenerator}
\GeneratedHydrator\ClassGenerator\{HydratorGenerator -> DefaultHydratorGenerator}
\GeneratedHydrator\Exception\{ExceptionInterface -> Exception}
\GeneratedHydrator\Exception\{DisabledMethodException::desabledMethod ->DisabledMethod::create}

@Ocramius
Copy link
Owner

@VolCh please also add the phpcs checks to .travis.yml 👍

@VolCh
Copy link
Contributor Author

VolCh commented May 12, 2018

@Ocramius should we maintain PHP 7.0 compatible or I shall take in mind #74 with PHP 7.1 at least?

@Ocramius
Copy link
Owner

Ocramius commented May 12, 2018 via email

@VolCh
Copy link
Contributor Author

VolCh commented May 12, 2018

@Ocramius I renamed classes

\GeneratedHydrator\ClassGenerator\\{
HydratorGeneratorInterface -> HydratorGenerator,
HydratorGenerator -> DefaultHydratorGenerator
}
\GeneratedHydrator\Exception\{
ExceptionInterface -> Exception,
DisabledMethodException::disabledMethod ->DisabledMethod::create
}

Also i'm not sure that \GeneratedHydrator\Configuration::DEFAULT_GENERATED_CLASS_NAMESPACE should be public

@VolCh
Copy link
Contributor Author

VolCh commented Jun 10, 2018

@Ocramius is this PR acceptable now?

*/
class HydratorGenerator implements HydratorGeneratorInterface
interface HydratorGenerator
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, this is a major BC break: would have preferred an exclusion rule, but let's roll with it

@Ocramius Ocramius self-assigned this Jun 10, 2018
@Ocramius Ocramius added this to the 3.0.0 milestone Jun 10, 2018
@Ocramius
Copy link
Owner

Requires a rebase, but otherwise good to go by me 👍

@VolCh VolCh force-pushed the master branch 2 times, most recently from d04c248 to 38f464f Compare June 11, 2018 06:24
@VolCh VolCh changed the title [wip] Apply Doctrine Coding Standard Apply Doctrine Coding Standard Jun 11, 2018
@VolCh
Copy link
Contributor Author

VolCh commented Jun 11, 2018

Done. Also fixed few @var phpdocs

@Ocramius Ocramius merged commit f009a17 into Ocramius:master Jun 11, 2018
@Ocramius
Copy link
Owner

👍 thanks!

@redthor
Copy link

redthor commented Sep 12, 2018

I don't suppose we could keep the library to php 7.1 for release 3.0.0?

When I trialled #79 I had to go to the 2.2.0 tag to be able to composer update (I could --ignore-platfrom-requirements but I was just thinking if there isn't something specifically in php7.2 we could keep it at php7.1...)

@Ocramius
Copy link
Owner

@redthor I bump my dependency versions regularly anyway: using an older stable version of the library is perfectly OK.

namespace GeneratedHydratorBenchmark;

/**
* Class that contains only private properties
*/
class AllPrivateClass
{
//phpcs:disable SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedProperty
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say it's preferable to do file-level excludes in phpcs.xml.dist rather than using inline comments.

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

Successfully merging this pull request may close these issues.

4 participants