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(bloc_lint): move bloc_lint package from vmichalak repository to bloc monorepo #4278

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vmichalak
Copy link
Collaborator

Status

READY

Breaking Changes

NO

Description

Move the bloc_lint from my personal repository to the bloc monorepo to simplify management of the package.

⚠️ i've had to add the plugin on the main analysis_option.yaml to make the example test folder works. Check if it's ok for you.

Implements: #4264

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@vmichalak vmichalak added the enhancement candidate Candidate for enhancement but additional research is needed label Nov 10, 2024
@vmichalak vmichalak self-assigned this Nov 10, 2024
@felangel
Copy link
Owner

@vmichalak thanks so much for the quick turnaround! I’ll take a closer look asap (I’m traveling right now). Can you also add me as a publisher to the pub package when you have a chance? Thanks!

@vmichalak
Copy link
Collaborator Author

@felangel i've add you to my publisher :)

@felangel
Copy link
Owner

@felangel i've add you to my publisher :)

I didn’t get the invite. Are you sure you added the right email? Mine is [email protected]

@vmichalak
Copy link
Collaborator Author

@felangel
image

@felangel
Copy link
Owner

@felangel image

Awesome just got it and moved the package under the bloclibrary publisher. I’ll try to review and get this PR merged later today (once I’m done traveling). Thanks so much for all the contributions! Looking forward to shipping an improved v0.2.0 in the near future 💙🎉

@vmichalak
Copy link
Collaborator Author

@felangel enjoy your traveling and we gonna talk about how to upgrade it for the 0.2.0 :)

Copy link

@zbarbuto zbarbuto left a comment

Choose a reason for hiding this comment

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

Can we maybe add a short description to each of the rules? I feel like lint rules are more valuable if they tell you why things are a good/bad idea. The official dart.dev lint rules do this, for example:

image

Something like:

avoid_public_method_on_bloc
Blocs are intended to be interacted with by adding events via the .add() method only. By exposing public methods it may be tempting to treat the bloc as if it were a cubit.

avoid_public_properties_on_bloc_and_cubit
Exposing public properties can lead to anti-patterns such as having public state that is not part of the state property or using repositories and services exposed by the bloc in the widget tree that should be accessed via other means such as RepositoryProvider.of() or context.read<T>()

prefer_multi_bloc_listener
MultiBlocListeners reduce nesting, make it easier to see what blocs are provided at the current widget tree level and can make refactoring easier.

@felangel
Copy link
Owner

Can we maybe add a short description to each of the rules? I feel like just having rules is slightly less valuable unless they actually tell you why things are a good/bad idea. The official dart.dev lint rules do this, for example:

image Something like:

avoid_public_method_on_bloc

  • Blocs are intended to be interacted with by adding events via the .add() method only. By exposing public methods it may be tempting to treat the bloc as if it were a cubit.

avoid_public_properties_on_bloc_and_cubit

  • Exposing public properties can lead to anti-patterns such as having public state that is not part of the state property or using repositories and services exposed by the bloc in the widget tree that should be accessed via other means such as RepositoryProvider.of() or context.read<T>()

prefer_multi_bloc_listener

  • MultiBlocListeners reduce nesting, make it easier to see what blocs are provided at the current widget tree level and can make refactoring easier.

Yup I think part of the v0.2.0 will be adding a docs page to the website explaining each lint rule. We first need to align on what the lint rules will be, what they’ll be named, and I also want to look into whether we can improve the developer experience and not require folks to add additional dependencies to use the lints.

@vmichalak
Copy link
Collaborator Author

not require folks to add additional dependencies to use the lints.

If you have any solution to add rules directly to the "dart analyzer" it could be awesome. But for the moment i have no idea of how to make this (that's why i've used custom_lint to make the first version).

@vmichalak
Copy link
Collaborator Author

@felangel For your info, i've add a pkg:bloc_lint label to create issues for the 0.2.0 of flutter_bloc. I gonna create tickets related to bloc_lint under this label.

image

@felangel
Copy link
Owner

@vmichalak I created #4281 to just create an empty scaffold so that we can discuss what specific lint rules we want to include rather than moving everything and then deleting/renaming. I figured it'd be easier and cleaner if we discuss and port over agreed upon changes instead. Hope that works for you, thanks again and sorry for the delay!

@vmichalak
Copy link
Collaborator Author

Since the first version is a proof of concept, seems ok to me let's work like that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement candidate Candidate for enhancement but additional research is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants