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 types to generated adapter read method #901

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Scowley4
Copy link

@Scowley4 Scowley4 commented Feb 8, 2022

Generated read method for adapters is currently missing types on two local variables (numOfFields and fields). This PR would add those types to the generated code.

@Scowley4 Scowley4 closed this Feb 8, 2022
@Scowley4 Scowley4 deleted the patch-1 branch February 8, 2022 20:59
@Scowley4 Scowley4 restored the patch-1 branch February 8, 2022 20:59
@Scowley4 Scowley4 reopened this Feb 8, 2022
@themisir
Copy link
Contributor

themisir commented Feb 9, 2022

Don't get me wrong but, this is opinionated change, some linter rules might require removing type annotations in above cases while some might want them to be there. In any case I would suggest ignoring linting on generated files if that's the case with you.

@Scowley4
Copy link
Author

Scowley4 commented Feb 9, 2022

@themisir Oh, that's interesting -- hadn't thought of that. I'm still pretty new to dart and I thought putting types in these places was "the standard".

You're probably right that I should ignore the generated files in my linter.

@themisir
Copy link
Contributor

themisir commented Feb 9, 2022

Yeah don't worry, it's alright. Dart linter is somewhat configurable using analyzer_options.yaml file. So some projects prefer all explicit type annotations while other ones prefer to not annotate types on obvious cases, I don't remember exact rule names but saw both rules used in different projects. I might be wrong tho, I haven't worked on dart project on last 3-4 months.

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.

2 participants