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

add initial rulesets #4

Merged
merged 2 commits into from
Apr 27, 2021
Merged

add initial rulesets #4

merged 2 commits into from
Apr 27, 2021

Conversation

pq
Copy link
Member

@pq pq commented Apr 27, 2021

@parlough
Copy link
Member

parlough commented Apr 27, 2021

Just as an unrelated question, once these are released, do you think you'll include them in the sets fields of the machine generated linter JSON?

@pq
Copy link
Member Author

pq commented Apr 27, 2021

@parlough: totally, we should do that. could you maybe open an issue on the linter to track?

@parlough
Copy link
Member

parlough commented Apr 27, 2021

@parlough: totally, we should do that. could you maybe open an issue on the linter to track?

For reference, opened an issue at dart-lang/linter#2611

@pq pq mentioned this pull request Apr 27, 2021
@@ -0,0 +1,28 @@
linter:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we want the file path here to be /lib/dart/core.yaml so that we could also have a /lib/flutter/core.yaml ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. Scoping has some other nice benefits too.

@goderbauer @devoncarew : wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

TBH I think that supporting frameworks in this package will be challenging. Once we add one there's no reason we shouldn't then support more; changes here will be tightly versioned together even though the frameworks will be logically unrelated.

I'd optimize for the 'dart' case here, and just have the two main files in the top level of the lib/ directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd optimize for the 'dart' case here,

I guess if we leave it as is, we could always add directories for frameworks (if we added them). So we might have something like:

lib/
 core.yaml
 recommend.yaml
 framework_1/
   core.yaml
   recommend.yaml
 framework_2/
   core.yaml
 ...

What do we think about flutter in the short term? Top-level, nested or somewhere else?

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I don't think we will have multiple lint sets for flutter. There'll just be a flutter.yaml set,

@pq
Copy link
Member Author

pq commented Apr 27, 2021

Landing and we can iterate.

Thanks for all the thoughtful back and forth!

@pq pq merged commit 9394d5a into main Apr 27, 2021
@pq pq deleted the init_rulesets branch April 27, 2021 23:52
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 18, 2024
* initial rulesets

* + include
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants