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

CI/Formatting #27

Merged
merged 6 commits into from
Oct 13, 2021
Merged

CI/Formatting #27

merged 6 commits into from
Oct 13, 2021

Conversation

kuhnroyal
Copy link
Collaborator

Wanted to get started on null-safety changes but there are constant format changes so I figured a consistent format and checks would help.

  • update workflow
  • extends analysis to all Dart code
  • add format check
  • format everything
  • enable dependabot for github actions

Copy link
Collaborator

@m0nac0 m0nac0 left a comment

Choose a reason for hiding this comment

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

Looks good apart from one minor note. Thank you!

On a side note: I'm pretty sure some of these files were previously formatted this way by flutter format, but maybe they changed the formatting rules at some point 😂

.github/workflows/flutter_ci.yml Outdated Show resolved Hide resolved
@m0nac0
Copy link
Collaborator

m0nac0 commented Oct 13, 2021

Sorry, the merge conflicts were created by #25 (removal of accessToken parameter, should be a quick fix, though)

@kuhnroyal
Copy link
Collaborator Author

On a side note: I'm pretty sure some of these files were previously formatted this way by flutter format, but maybe they changed the formatting rules at some point 😂

Yea something changed in recent Dart versions, should probably also use a fixed Flutter version in the CI script?

@kuhnroyal
Copy link
Collaborator Author

Rebased, hope I didn't miss anything.

@m0nac0
Copy link
Collaborator

m0nac0 commented Oct 13, 2021

Yea something changed in recent Dart versions, should probably also use a fixed Flutter version in the CI script?

Hm, I thought about this. That would also allow caching the flutter installation. But then someone would need to think about updating that fixed version regularly. So I'd prefer to continue without a fixed flutter version. And if the formatting rules really change, it's easy to reformat.

@kuhnroyal
Copy link
Collaborator Author

Yea something changed in recent Dart versions, should probably also use a fixed Flutter version in the CI script?

Hm, I thought about this. That would also allow caching the flutter installation. But then someone would need to think about updating that fixed version regularly. So I'd prefer to continue without a fixed flutter version. And if the formatting rules really change, it's easy to reformat.

Could setup a matrix build with fixed stable and beta.

@m0nac0
Copy link
Collaborator

m0nac0 commented Oct 13, 2021

Could setup a matrix build with fixed stable and beta.

Interesting idea, feel free to submit a PR if you'd like to.

Two more ideas, if we're already at it:

  1. we could run the CI (with the flutter beta branch) in regular intervals, so that any changes in the flutter sdk breaking this plugin are more easily detected, even if there is no active PR.
  2. we could integrate the formatting check into the static analysis job. It's possible to configure the step in a way that the linter would always run, even if the format check returns an error. That would save Github/the environment about 2mins of execution time per CI run.

@m0nac0 m0nac0 merged commit 17b4ab3 into maplibre:main Oct 13, 2021
@kuhnroyal kuhnroyal deleted the feature/formatting branch October 13, 2021 18:10
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