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

Italian basics Rules #197

Open
wants to merge 5 commits into
base: 2.0.x
Choose a base branch
from
Open

Italian basics Rules #197

wants to merge 5 commits into from

Conversation

riettotek
Copy link

I wrote the basics. I'm italian.
I'm already using these rules for migrations. If u want to include them by reviewing . Thank u :)

Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Grazie mille per il lavoro iniziale @riettotek! 🇮🇹

Apart from few comments I've left, the PR must meet our standards before merging:

  1. pass all the checks (the coding style can be fixed by running vendor/bin/phpcbf)
  2. have tests that prove your code is working as expected and preventing regressions in the future

{
yield new Transformation(new Pattern('e$'), 'a');
yield new Transformation(new Pattern('i$'), 'e');
yield new Transformation(new Pattern('i$'), 'o');
Copy link
Member

Choose a reason for hiding this comment

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

This transformation will never happen given the previous one has exactly same catch pattern

yield new Transformation(new Pattern('a$'), 'e');
yield new Transformation(new Pattern('e$'), 'i');
yield new Transformation(new Pattern('io$'), 'i');
yield new Transformation(new Pattern('o$'), 'i');
Copy link
Member

Choose a reason for hiding this comment

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

These two rules could be merged into new Pattern('i?o$') I believe

*/
public static function getIrregular() : iterable
{
return yield new Substitution(new Word(''), new Word(''));
Copy link
Member

Choose a reason for hiding this comment

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

This looks unfinished

@@ -0,0 +1,38 @@
# doctrine-Italian-inflector-
Copy link
Member

Choose a reason for hiding this comment

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

This file seems irrelevant once we merge the PR

{
yield from self::getDefault();

yield new Pattern('.*ss');
Copy link
Member

Choose a reason for hiding this comment

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

These words looks like copy-pasted from English

yield new Pattern('equipment');
yield new Pattern('evidence');
yield new Pattern('feedback');
yield new Pattern('food');
Copy link
Member

Choose a reason for hiding this comment

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

Definitely

@f-liva f-liva mentioned this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants