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

feat(lint): add stylelint overrides and variation files #2593

Merged
merged 32 commits into from
Dec 10, 2022

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Dec 10, 2022

In #2592 I completely missed there are also LESS files named with *.overrides and *.variables extensions, so these LESS files are fixed in this PR. As in #2592, only whitespaces are fixed, no functional change.

@mvorisek
Copy link
Contributor Author

What about renaming (in another PR) *.overrides -> *.overrides.less (and vice versa for *.variables)?

@lubber-de
Copy link
Member

lubber-de commented Dec 10, 2022

What about renaming (in another PR) *.overrides -> *.overrides.less (and vice versa for *.variables)?

This would possibly break tons of existing projects dealing with custom themes . I would not dare to do that for FUI 2.x

@mvorisek mvorisek marked this pull request as ready for review December 10, 2022 18:28
@mvorisek
Copy link
Contributor Author

What about renaming (in another PR) *.overrides -> *.overrides.less (and vice versa for *.variables)?

This would possibly break tons of existing projects dealing with custom themes . I would not dare to do that for FUI 2.x

very valid argument 👍

PR is complete, minified css hash is the same

@@ -16,5 +16,5 @@

@flags: {
};
@size-map: {
};
Copy link
Member

Choose a reason for hiding this comment

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

Dont remove the semicolon here, to support PHP LESS port

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was removed by no-missing-end-of-source-newline rule, not sure why, checking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can confirm this was removed by c877b76 autofix, when the rule is fixed manually, the semicolon stays. Other fixes by no-missing-end-of-source-newline are fine (no semicolon removed).

Copy link
Member

@lubber-de lubber-de left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de lubber-de changed the title feat(lint): add stylelint for LESS and normalize whitespaces II. feat(lint): add stylelint overrides and variation files Dec 10, 2022
@lubber-de lubber-de merged commit ce26829 into fomantic:develop Dec 10, 2022
@lubber-de lubber-de added this to the 2.9.1 milestone Dec 10, 2022
@lubber-de lubber-de added lang/css Anything involving CSS type/chore Anything which is a project chore type/ci Anything related to CI/CD labels Dec 10, 2022
@mvorisek mvorisek deleted the more_stylelint branch December 10, 2022 21:09
@lubber-de lubber-de added the type/lint eslint / stylelint related changes only label Dec 12, 2022
lubber-de pushed a commit that referenced this pull request Dec 21, 2022
Compared to Stylelint, Prettier is very simple tool and I am quite supprised Stylelint is deprecating the whitespace rules in favor of Prettier. For LESS it seems ok, for JS, we should never enable Prettier as formatting is part of a code and improves readability.

As Prettier is an opinionated formatter instead of an linter, we need to satisfy it everywhere without exceptions.

When I was crafting this PR, I found several Prettier issues and proposed changes to Prettier:

Fix semicolon duplicated at the end of LESS file prettier/prettier#14007
Fix no space after unary minus when followed by opening parenthesis in LESS prettier/prettier#14008
Do not change case of property name if inside a variable declaration in LESS prettier/prettier#14034
In this PR, these changes are contained and Prettier is patched before it is run. Once the changes are merged in prettier and stable release is made, they can be removed.

This PR fixes minor whitespaces unfixed/undetected in GH-2610.

And also asserts:

PHP port of the LESS fails to compile colors.less #1832
feat(lint): add stylelint overrides and variation files #2593 (comment)
Prettier has no built in support to dump the diff - prettier/prettier#6885 - so we dump it in the CI using git.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/css Anything involving CSS type/chore Anything which is a project chore type/ci Anything related to CI/CD type/lint eslint / stylelint related changes only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants