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: restructured, cleaned, added pub.dev checks #108

Merged
merged 10 commits into from
Dec 19, 2021

Conversation

daadu
Copy link
Member

@daadu daadu commented Dec 19, 2021

  • [deploy_demo]: proper step name, switched to "stable"
  • [pr_title]: proper step name

- [deploy_demo]: proper step name, switched to "stable"
- [pr_title]: proper step name
@daadu daadu requested a review from WieFel December 19, 2021 09:28
@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

@WieFel Will release v0.7.0 after this merge.

… job

- cd: using scripts to install - flutter and dependencies
- install flutter at $HOME - instead of inside project
@daadu daadu requested a review from WieFel December 19, 2021 10:09
@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

@WieFel Can you review now.

@WieFel
Copy link
Collaborator

WieFel commented Dec 19, 2021

@daadu what I find a bit strange is using .sh files for installing flutter + dependencies. Why did you come up with that solution, instead of using existing github actions that can be included?

@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

@daadu what I find a bit strange is using .sh files for installing flutter + dependencies. Why did you come up with that solution, instead of using existing github actions that can be included?

Mainly to make the step faster - check new - 59s vs old - 66s, although the difference is not much, I expected more though!

@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

I was expecting the 3rd party action to be using caching - for flutter binary - but looks like they don't. May be with our script can add caching to make it faster.

@WieFel
Copy link
Collaborator

WieFel commented Dec 19, 2021

@daadu what I find a bit strange is using .sh files for installing flutter + dependencies. Why did you come up with that solution, instead of using existing github actions that can be included?

Mainly to make the step faster - check new - 59s vs old - 66s, although the difference is not much, I expected more though!

In my opinion, readability/clarity of the actions would be more important in this case than 5s of quicker execution.
But of course, if you prefer it this way it is also fine with me ;) you have the final word.

- removed `install-flutter.sh` and `install-dependencies.sh`
@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

In my opinion, readability/clarity of the actions would be more important in this case than 5s of quicker execution. But of course, if you prefer it this way it is also fine with me ;) you have the final word.

going with your suggestion. makes sense.

@daadu daadu changed the title ci: proper naming and cleanup ci: restructured, cleaned, added pub.dev checks Dec 19, 2021
@daadu daadu merged commit f6687ec into fluttercommunity:master Dec 19, 2021
@daadu daadu deleted the ci-cleanup branch December 19, 2021 10:52
@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

@WieFel The total difference on run is of 30s - current vs previous

@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

We can try adding caching to current to reduce time or come back to git clone approach. Anyways not important for now, just for note - if in future the CI becomes slower - we might have to think on it.

@WieFel
Copy link
Collaborator

WieFel commented Dec 19, 2021

@daadu ok!
Yeah I think 30s difference is ok for now. Github actions are actually not trimmed for running very fast anyway. Their purpose is to run on some events and give feedback, regardless of needing few seconds more or less 😉

@daadu
Copy link
Member Author

daadu commented Dec 19, 2021

I think CI feedback should be fast though - like in about a 1-2m should know things are ok!

@WieFel
Copy link
Collaborator

WieFel commented Dec 19, 2021

Yeah sure! As long as it is within this limit it is ok

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