Skip to content

Conversation

M123-dev
Copy link
Contributor

@M123-dev M123-dev commented Oct 7, 2021

For #87 cf: #86

I updated some further linting rules and updated the code accordingly, for an easier review process I've put the different types of changes into different commits (with a description of what the change was)

This class (or a class that this class inherits from) is marked as '@immutable', but one or more of its instance fields aren't final: WorkoutForm._plan
info: Prefer const with constant constructors. prefer_const_constructors
@M123-dev
Copy link
Contributor Author

M123-dev commented Oct 7, 2021

Now only four hints are left in the analyzer:
Unbenannt

I dont know if the not used vars are planned to be used so I left them in and the other two are some flutter things which got deprectaed lately but I left them in for now since we are using a hybrid flutter version.


static const daysToCache = 7;

static const _exerciseInfoUrlPath = 'exerciseinfo';
Copy link
Member

Choose a reason for hiding this comment

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

quick question, why did you remove the underscore? Since this isn't a library, we know that we'll be not misusing methods or properties but if it's clear they are only meant for "internal" consumption, why not mark them as such?

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 myself was confused why the analyzer complained about it. But it turns out that static variables cannot be private. (Couldn't find anything online about it but a short dartPad test proved this thesis). We can of course remove the static modifier instead of the _, but at that time I haven't thought about that.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhhh. I guess it kinda makes sense.

And in our case, in the end it doesn't really matter, they are constants anyway

@rolandgeider
Copy link
Member

thanks again for another herculean PR ;)

the ones for the exercises will be removed, we're refactoring that part in another branch anyway. If the other one isn't used then they can be removed

@rolandgeider rolandgeider linked an issue Oct 10, 2021 that may be closed by this pull request
@M123-dev
Copy link
Contributor Author

M123-dev commented Oct 11, 2021

@rolandgeider I think I haven't understood you right, should I undo the changes in the exercises or will they just be overwritten

Also we had merge conflicts in the pubspec.lock file I fixed them manually, hopefully nothing broke

Edit: CI came to the tests so we should be fine

@rolandgeider
Copy link
Member

I meant that we can remove the exercises variables since they will be reworked anyway, I can do that

@M123-dev
Copy link
Contributor Author

@rolandgeider you should have write access to my branch feel free to edit things

@rolandgeider
Copy link
Member

Will do, thanks!

@rolandgeider rolandgeider merged commit 4ad5263 into wger-project:master Oct 13, 2021
@M123-dev M123-dev deleted the better-linting-rules-1 branch October 13, 2021 18:50
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.

Use flutter_lints package
2 participants