Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

feat: add liferay/no-it-should rule #43

Merged
merged 1 commit into from
Jun 27, 2019
Merged

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Jun 27, 2019

Implements enforcement of:

https://github.com/liferay/liferay-frontend-guidelines/blob/master/guidelines/general/testing.md

As noted in the comments, ESLint surprisingly doesn't make it easy for you to bundle rules with a config, requiring you to publish a separate package. That is, if you have "eslint-config-foo", you have to stick your rules in "eslint-plugin-foo". The inverse doesn't appear to be true: if you have "eslint-plugin-foo", you can bundle configs with it without having to create "eslint-config-foo".

We however, have "eslint-config-liferay", and I don't want to create yet another package, so we use a horrifying but well-contained hack that is hidden away in utils/local.js to enable us to bundle local rules with our config. No need to look in there, really.

So, we now have a "plugins" subdirectory where we keep our local rules, starting with this "liferay/no-it-should" rule, which we should be able to use in all of our projects, and which you can see in "plugins/eslint-plugin-liferay". The actual structure of this is determined by the "generator-eslint" Yeoman generator, which is why it doesn't exactly follow our own conventions (eg. it has a "tests" directory in instead of a "test" directory). Likewise, things like having a "docs" folder are just expected for all ESLint rules. I included a link from the top-level README for visibility.

Implements enforcement of:

https://github.com/liferay/liferay-frontend-guidelines/blob/master/guidelines/general/testing.md

As noted in the comments, ESLint surprisingly doesn't make it easy for
you to bundle rules with a config, requiring you to publish a separate
package. That is, if you have "eslint-config-foo", you have to stick
your rules in "eslint-plugin-foo". The inverse doesn't appear to be
true: if you have "eslint-plugin-foo", you can bundle configs with it
without having to create "eslint-config-foo".

We however, have "eslint-config-liferay", and I don't want to create yet
another package, so we use a horrifying but well-contained hack that is
hidden away in `utils/local.js` to enable up to bundle local rules with
our config. No need to look in there, really.

So, we now have a "plugins" subdirectory where we keep our local rules,
starting with this "liferay/no-it-should" rule, which we should be able
to use in all of our projects, and which you can see in
"plugins/eslint-plugin-liferay". The actual structure of this is
determined by the "generator-eslint" Yeoman generator, which is why it
doesn't exactly follow our own conventions (eg. it has a "tests"
directory in instead of a "test" directory). Likewise, things like
having a "docs" folder are just expected for all ESLint rules. I
included a link from the top-level README for visibility.
@wincent
Copy link
Contributor Author

wincent commented Jun 27, 2019

Follow-up to this will be adding a "liferay-portal/no-side-navigation" rule inside "plugins/eslint-plugin-liferay-portal", and accessible by extending the "liferay/portal" preset, which will show how we can create liferay-portal specific rules.

To @jbalsas' offline question about why we'd want those rules here instead of in liferay-portal itself, I think that there's some value in keeping everything in one place, at least to start with. (Ability to see/develop/test all the lints together.) We can revisit that decision later if we run into problems with it.

@jbalsas
Copy link
Contributor

jbalsas commented Jun 27, 2019

Looks good to me! I'd like to se the associated guidelines tickets (if any) getting resolved containing a link to the rules in here when it gets merged. Not sure if we can automate that somehow... maybe it's a manual process we need to deal with anyways.

@wincent wincent merged commit a472e9b into master Jun 27, 2019
@wincent wincent deleted the wincent/no-it-should branch June 27, 2019 09:07
@wincent
Copy link
Contributor Author

wincent commented Jun 27, 2019

I added a link back to the lint from liferay-frontend-guidelines here. Probably no way to automate this at this point, as some lints have corresponding GitHub issues, others have Jira tickets, and others still have neither (for better or for worse).

wincent added a commit that referenced this pull request Jun 27, 2019
My previous PR (#43) showed how to add a generic rule that we apply
everywhere (both inside liferay-portal and in our open source repos).

This PR shows how we add rules specific to liferay-portal; it:

- Bundles a new "eslint-plugin-liferay-portal" inside this package.
- Adds a "liferay-portal/no-side-navigation" rule.
- Adds a new preset config "liferay/portal" (joining the "liferay/react"
  variant that we already had), which uses the new rule.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants