feat: lint direct require quo component outside src/quo #17828
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (19)
|
flexsurfer
reviewed
Nov 6, 2023
yqrashawn
force-pushed
the
feat/lint-quo-component-outside-quo
branch
5 times, most recently
from
5a06921
to
6b40c8e
Compare
yqrashawn
commented
Nov 6, 2023
| @@ -21,7 +21,7 @@ let | |||
| buildInputs = with pkgs; [ | |||
| clojure flock maven openjdk | |||
| # lint specific utilities | |||
| clj-kondo zprint clojure-lsp | |||
| clj-kondo zprint clojure-lsp ripgrep | |||
There was a problem hiding this comment.
Hi @jakubgs, is it ok to put this here? Or into the default shell.
yqrashawn
force-pushed
the
feat/lint-quo-component-outside-quo
branch
from
6b40c8e
to
f5151b2
Compare
J-Son89
approved these changes
Nov 6, 2023
| # quo components namespace ends with style|utils|types|reaction-resource | ||
| # are not the component view, they are usually utils fns or styles | ||
|
|
||
| if rg --pcre2 --glob '!src/quo/**/*' 'quo\.components(\.[\w-]+)*\.[\w-]*(?<!style|utils|types|reaction-resource)(\s|\]|\n)' src; then |
There was a problem hiding this comment.
I'm not sure of these other file types, but if someone uses a quo component's style.cljs file outside of the quo namespace that should be a big code smell imo. I don't think it's something we need to lint for however 👍
There was a problem hiding this comment.
These are usages I filterd out
First 3 are used in old profile screen to pass new look into old components. I added these 3.
I guess we will remove these when new profile screen added.
2 of them are used in quo_preview. I'm not sure, maybe it's not required when using outside of preview
one is in comment
these 2 are indeed a bit weird
src/status_im2/contexts/emoji_picker/style.cljs
src/status_im2/contexts/chat/messages/drawers/view.cljs
There was a problem hiding this comment.
Oh let's lint this so, we really shouldn't have these uses imo
There was a problem hiding this comment.
Or at least create a follow up issue to address this
flexsurfer
approved these changes
Nov 6, 2023
yqrashawn
force-pushed
the
feat/lint-quo-component-outside-quo
branch
from
f5151b2
to
f1fa87e
Compare
jakubgs
approved these changes
Nov 6, 2023
Signed-off-by: yqrashawn <namy.19@gmail.com>
yqrashawn
force-pushed
the
feat/lint-quo-component-outside-quo
branch
from
f1fa87e
to
544bdc5
Compare
yevh-berdnyk
pushed a commit
that referenced
this pull request
Dec 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
make lintto lint against require quo component outsidesrc/quo/appfolder for prettiererror looks like
Testing notes
dev exp only PR
status: ready