search

ns-rx / nataliesmith

0 stars 0 forks source link

PHPCS config suggestions #3

Closed GaryJones closed 1 month ago

GaryJones commented 1 month ago

Some suggestions for the phpcs.xml.dist file.

Remove duplicate entries

These two excludes: https://github.com/ns-rx/nataliesmith/blob/b18dfc82338d03877475a200501366e5f315bafe/phpcs.xml.dist#L28-L32 ... are effectively duplicates of the earlier excludes. They can be removed.

Remove redundant *.min.js exclusion

The extensions config https://github.com/ns-rx/nataliesmith/blob/b18dfc82338d03877475a200501366e5f315bafe/phpcs.xml.dist#L15 ...means that a .js file (and .min.js) file won't be scanned anyway, so the exclusion is redundant.

Remove or update directory exclusions.

Right now, only one child theme directory is due to be scanned, so the exclusions for the top-level /vendor/, /wp/, and /node_modules/ are redundant and could be removed.

However, you may want to consider updating them to include a leading asterisk, so that */vendor/, and */node_modules/ that appear anywhere in the repo are not scanned, such as those that may be found in the child theme directory or future plugin directories.

ns-rx commented 1 month ago

Great suggestions, thank you!

GaryJones commented 1 month ago

When doing commits / PRs, be sure to mention the issue number - see https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue but it applies to commit messages and comments on commits too (see my example).

Be sure to get into the habit of writing yourself PRs instead of committing to the main branch directly - it's a bit more effort, but it provides a cleaner tracking of work, and allows reviews to happen, CI tasks to run, and can more easily be reverted if needed.

ns-rx commented 1 month ago

Brilliant, will do!