Coding standard compliance

[indigoxela]

Coding standard compliance

[indigoxela]

All those missing parameter types will be a bunch of work - it's sometimes hard to tell what the right type is in those nested functions... especially i18n_string is pretty complex.

[indigoxela]

I've left comments on your current PR.

A quick idea: instead of blocking phpcs on files containing classes with underscores even for local phpcs runs (via @codingStandardsIgnoreFile), we could exclude them on the command line run in travisCI.

That way people don't get a test failure on things no one can fix in 1.x, but it's still possible to keep an eye on other violations when run locally.

@serundeputy what do you think?

Affected files:

• i18n_object.inc
• i18n_translation/i18n_translation.inc
• i18n_taxonomy/i18n_taxonomy.inc
• i18n_path/i18n_path.inc
• i18n_string/i18n_string.inc
• i18n_menu/i18n_menu.inc
• i18n_field/i18n_field.inc

[indigoxela]

Done. It's impossible to fully comply the standards, though, for instance we can't simply rename existing classes and methods (camelcase).

[indigoxela]

Reopening this as @serundeputy was so kind to provide a .travis.yml config in a new PR:

#25

[serundeputy]

Done. It's impossible to fully comply the standards, though, for instance we can't simply rename existing classes and methods (camelcase).

• for these cases we can/should add

// @TODO: Rename classes to CamelCase in 2.x.
// @codingStandardsIgnoreLine

[indigoxela]

Regarding @codingStandardsIgnoreLine: we'd need a lot of those, so I'm undecided yet.

Newer phpcs versions would allow to --exclude sniffs via command line, but backdrop/coder requires old 1.5. Ever thought about upgrading? I'm aware this requires rewriting sniffs.

[serundeputy]

I think about upgrading ... I originally pinned that version to support php 5.3, but these days i'm less concerned about that, but worth noting that backdrop core still supports back to php5.3.

[serundeputy]

The travis builds/tests are not running. I'd recommend we go through and file PRs per directory.
i18n_forum; PR #26

[indigoxela]

Nope, we have submodules, that should get completely ignored.

See #1 for details.

In other words: don't waste time on something that we don't actually need. 😉

Submodules to ignore:

• i18n_block
• i18n_forum
• i18n_select
• i18n_user
• i18n_variable

The reason why they're still there is to figure out a migration path from D7.

[serundeputy]

ahh; ok; let me see about telling travis not to check those

[serundeputy]

ok; assuming these are legit failures: https://travis-ci.org/github/backdrop-contrib/i18n/builds/679360294

[serundeputy]

That's a great idea!

[indigoxela]

Although coding standards will be an ongoing task, I'm closing this issue for now, as travisCI test is green.