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.

Sign up for GitHub

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

Jump to bottom

Enable staticcheck and fix all issues #144

Merged
merged 7 commits into from
Dec 16, 2020
Merged

Enable staticcheck and fix all issues #144

merged 7 commits into from
Dec 16, 2020

Conversation

Jacalz
Copy link
Member

@Jacalz Jacalz commented Oct 24, 2020 •
edited
Loading

This PR makes sure to get all the issues reported by staticcheck fixed and marks the ones that can't be fixed to be ignored for now (waiting for Fyne 2.0.0 mostly). It should now be possible to set the GitHub action to be required 👍

Jacalz
Jacalz commented Oct 24, 2020
View reviewed changes
screen.go Show resolved Hide resolved
internal/ui/widgetpanel.go Outdated Show resolved Hide resolved
@Jacalz Jacalz changed the title Staticcheck Enable staticcheck and fix all issues Oct 24, 2020
@Jacalz
Copy link
Member Author

Jacalz commented Oct 24, 2020

Staticcheck is being killed for some strange reasons. Was this what you were seeing before you turned off staticcheck? @andydotxyz

@andydotxyz
Copy link
Contributor

Was this what you were seeing before you turned off staticcheck? @andydotxyz

Yes

@Jacalz
Copy link
Member Author

Jacalz commented Oct 24, 2020

@dominikh Do you have any idea as to why this would be happening to staticcheck?

@dominikh
Copy link

dominikh commented Oct 24, 2020 •
edited
Loading

It's being killed because of an out of memory situation. This is probably dominikh/go-tools#825.

It's likely that some package in Fyne is triggering bad behavior in Staticcheck and causes way higher memory usage than is normal. Diagnosing this is still on my todo list.

andydotxyz
andydotxyz reviewed Oct 28, 2020
View reviewed changes
screen.go Show resolved Hide resolved
andydotxyz
andydotxyz requested changes Nov 2, 2020
View reviewed changes
Copy link
Contributor

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

I think one or two of my previous comments should have been "request changes" so applying that now

andydotxyz
andydotxyz requested changes Nov 19, 2020
View reviewed changes
internal/ui/baricon.go Outdated Show resolved Hide resolved
internal/x11/win/frame.go Show resolved Hide resolved
@andydotxyz
Copy link
Contributor

I think that if this was rebased the checks would start running again

@Jacalz
Copy link
Member Author

Jacalz commented Dec 11, 2020

It should now be updated against latest develop again. Going to see if I can fix these comments.

@Jacalz Jacalz requested a review from andydotxyz December 11, 2020 18:42
@Jacalz Jacalz mentioned this pull request Dec 11, 2020
Merged
@Jacalz
Copy link
Member Author

Jacalz commented Dec 11, 2020

Looks like we are still seeing staticcheck getting killed.

@andydotxyz
Copy link
Contributor

So #148 would fix this maybe?

@Jacalz
Copy link
Member Author

Jacalz commented Dec 15, 2020

Hopefully. Lets rebase on it when that one lands 🙂

@Jacalz Jacalz merged commit 397915e into FyshOS:develop Dec 16, 2020
@Jacalz Jacalz deleted the staticcheck branch December 16, 2020 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andydotxyz andydotxyz andydotxyz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jacalz @andydotxyz @dominikh