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

cmd/fyne: Switch to using strings for bundled resources #1638

Merged
merged 4 commits into from Dec 12, 2020
Merged

cmd/fyne: Switch to using strings for bundled resources #1638

merged 4 commits into from Dec 12, 2020

Conversation

Jacalz
Copy link
Member

@Jacalz Jacalz commented Dec 8, 2020

Description:

This moves on with moving to strings over []byte for bundled items. Should decrease memory usage in gopls even more and for applications using bundled resources. Hopefully this can get fynedesk staticcheck to not die from memory usage.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

@lucor
Copy link
Member

lucor commented Dec 8, 2020

Wondering if an ignore directive for staticcheck could make sense since these are autogenerated files. I don't know if exists something similar for gopls that could help.

@andydotxyz
Copy link
Member

Wondering if an ignore directive for staticcheck could make sense since these are autogenerated files

Not rreally - we are generating the files, so should generate them correctly. That is what @Jacalz change does I think.

@Jacalz
Copy link
Member Author

Jacalz commented Dec 9, 2020

It looks like staticcheck doesn’t have support for that yet. You can turn off specific checks for a file but not exclude them fully. dominikh/go-tools#429

@lucor
Copy link
Member

lucor commented Dec 9, 2020

I see. I was thinking only some types of checks was causing problem, hence the idea to have the generator to add a linter ignore directive.

@andydotxyz
Copy link
Member

we are generating the files, so should generate them correctly

I think this still holds?

@lucor
Copy link
Member

lucor commented Dec 9, 2020

we are generating the files, so should generate them correctly

I think this still holds?

Agreed generated files should be generated correcly. Just wondering if an accurate and manual check on the generated files could be good enough for now, since we probably won't change the generator often and the static analisys on the generated files is causing trouble :)

@Jacalz
Copy link
Member Author

Jacalz commented Dec 9, 2020

Given that staticcheck doesn't support disabling for whole files (going through and testing the individual checks seems time consuming) and we already apply // auto-generated to the file, I think we should be fine.

@andydotxyz
Copy link
Member

static analisys on the generated files is causing trouble :)

I don't think this is the case... this PR changed the code that generates the files, and the output was not correct.
By adjusting it further the output will be correct again.

andydotxyz
andydotxyz previously approved these changes Dec 10, 2020
@Jacalz
Copy link
Member Author

Jacalz commented Dec 11, 2020

Sorry @andydotxyz. You will need to re-approve now that I have rebased on latest develop (release 1.4.x merge related).

@Jacalz Jacalz merged commit fcf0c4d into fyne-io:develop Dec 12, 2020
@Jacalz Jacalz deleted the stage2-string-bundle branch December 12, 2020 09:58
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.

None yet

3 participants