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

chore: update sdk version, yaru and add CI and release workflows #1

Merged
merged 13 commits into from
Oct 30, 2024
Merged

chore: update sdk version, yaru and add CI and release workflows #1

merged 13 commits into from
Oct 30, 2024

Conversation

Feichtmeier
Copy link
Member

Updated:

  • sdk version
  • dependencies including...
  • ... yaru
  • remove dold yaru libs
  • fixed analyzer warnings
  • added todo for dead code I don't know if it is needed

Added:

  • CI workflow
  • release workflow

@Feichtmeier Feichtmeier requested a review from HrX03 October 28, 2024 18:38
@Feichtmeier Feichtmeier requested a review from larsb24 October 28, 2024 18:45
HrX03
HrX03 requested changes Oct 28, 2024
View reviewed changes
Copy link
Member

@HrX03 HrX03 left a comment

Choose a reason for hiding this comment

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

looks pretty good, mostly stylistic comments

clipboard_viewer/lib/main.dart Outdated Show resolved Hide resolved
lib/backend/database/helper.dart Outdated Show resolved Hide resolved
lib/backend/entity_info.dart Outdated Show resolved Hide resolved
lib/backend/fetch.dart Outdated Show resolved Hide resolved
lib/backend/fetch.dart Outdated Show resolved Hide resolved
lib/backend/path_parts.dart Outdated Show resolved Hide resolved
lib/backend/stat_cache_proxy.dart Outdated Show resolved Hide resolved
lib/widgets/table.dart Outdated Show resolved Hide resolved
@Feichtmeier Feichtmeier force-pushed the chore/update_dependencies branch from a17770a to e56ab06 Compare October 29, 2024 07:49
larsb24
larsb24 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Member

@larsb24 larsb24 left a comment

Choose a reason for hiding this comment

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

Changes look good to me!

@Feichtmeier
Copy link
Member Author

@HrX03 you only marked some of the changes dart fix --apply did to constructor placements. do you want me to move them all back? 😮‍💨

@HrX03
Copy link
Member

HrX03 commented Oct 30, 2024

@HrX03 you only marked some of the changes dart fix --apply did to constructor placements. do you want me to move them all back? 😮‍💨

If possible yes, sorry!!

@Feichtmeier
Copy link
Member Author

Feichtmeier commented Oct 30, 2024 •
edited
Loading

the problem is that the default behaviour of code generated by flutter places the constructor first

Bildschirmaufnahme.2024-10-30.um.14.38.41.mov

don't you think it makes sense to follow flutter for common lints and practices for a flutter app?

flutter/flutter#1220 (comment)

@HrX03
Copy link
Member

HrX03 commented Oct 30, 2024

Don't really like that behaviour either but I see your point, don't know, I'd still prefer to have it the other way around but even the way it is should be okay either way. The only thing I ask then is to have a new line between the constructor and the fields, I'll accept the pr with the new order

@Feichtmeier
Copy link
Member Author

Ping pingy @HrX03

@Feichtmeier Feichtmeier merged commit 3b03deb into master Oct 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larsb24 larsb24 larsb24 approved these changes

@HrX03 HrX03 HrX03 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
@Feichtmeier @HrX03 @larsb24