Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

Update dependencies #102

Merged
merged 11 commits into from
Aug 5, 2020

Conversation

techno-disaster
Copy link
Contributor

@techno-disaster techno-disaster commented Jun 16, 2020

Description

Update dependencies and put beta in github action instead of hardcoding flutter version

Flutter Channel:

  • I have used the Flutter Beta channel on my local machine

How has this been tested?

Logs with these changes can be found here

Type of Change:

Delete irrelevant options.

  • Quality Assurance

@techno-disaster techno-disaster added Category: Quality Assurance Changes to code or files that improve testing or fixes bugs. Status: Needs Review PR needs an additional review or a maintainer's review. Type: Maintenance Repository maintenance. labels Jun 16, 2020
@bartekpacia
Copy link
Contributor

bartekpacia commented Jun 16, 2020

Won't it create non-deterministic builds?

Also: commit squashing ;P

@techno-disaster
Copy link
Contributor Author

Won't it create non-deterministic builds?

Also: commit squashing ;P

I'm not sure what non-deterministic builds are ?
also we have squash and merge enabled ;)

@bartekpacia
Copy link
Contributor

bartekpacia commented Jun 16, 2020

I mean that the Flutter version we're using should be explicitly defined.
The project might work well on your local machine but crash at the CI stage because of different Flutter version.

PS After writing this, I'm not sure whether it is a good or bad idea. Non-deterministic builds are a problem on Android when you don't hardcode libraries version. This case is slightly similar but also slightly different.

@techno-disaster
Copy link
Contributor Author

I mean that the Flutter version we're using should be explicitly defined.
The project might work well on your local machine but crash at the CI stage because of different Flutter version.

PS After writing this, I'm not sure whether it is a good or bad idea. Non-deterministic builds are a problem on Android when you don't hardcode libraries version. This case is slightly similar but also slightly different.

but if we have already asked contributors to work with the latest flutter beta branch (this is a check in issue template too) , won't the local machine flutter version and beta branch version be the same? Because basically beta channel is cloning the flutter repo and the beta branch.You can check this out here

@bartekpacia
Copy link
Contributor

We can't assume this, because people often forget to flutter upgrade.

@techno-disaster
Copy link
Contributor Author

techno-disaster commented Jun 16, 2020

We can't assume this, because people often forget to flutter upgrade.

Well in that case, even if we update our CI with hard-coded version number and the contributor hasn't updated it will be the same case right? for example flutter released v2 for beta, and we updated it on the CI, but the contributor forgot flutter upgrade, this will be the same case as you are saying right? Solution to this could be to just stick with a very old flutter beta build to be sure everyone is on that atleast, but that doesn't make any sense right? So what do you think, how should we handle this?
also the Travis CI has been using the beta branch from the beginning, and we haven't faced any issue with it yet

@bartekpacia
Copy link
Contributor

Okay, I think we can try this and see if we'll encounter any problems.

robotjellyzone
robotjellyzone previously approved these changes Jul 9, 2020
Copy link

@robotjellyzone robotjellyzone left a comment

Choose a reason for hiding this comment

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

@techno-disaster Since these were slight changes so, it Looks good to me 👍 but to check whether these changes are giving required result this needs testing to changing the label to Testing

Also, can you please resolve the merge conflicts? @techno-disaster

@robotjellyzone robotjellyzone added Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Jul 9, 2020
bartekpacia
bartekpacia previously approved these changes Jul 9, 2020
@techno-disaster
Copy link
Contributor Author

Thanks for solving the conflict @bartekpacia . Keeping bloc and flutter bloc on version 4.x.x because some changes are required for migrating to 5.x.x , Will do that separately in another PR.
Can someone from @anitab-org/qa-team give this a test? Just the dependencies were updated no new features/ bug fixes

Copy link

@robotjellyzone robotjellyzone left a comment

Choose a reason for hiding this comment

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

I have tested every other feature of the app, so, everything is working and nothing is breaking as expected as can be seen from the following gif :

pr-jayesh

great job @techno-disaster 👍

@robotjellyzone
Copy link

Just want to know is Find Members feature is not implemented yet ? or maybe I am not aware of it? @techno-disaster

@techno-disaster
Copy link
Contributor Author

@robotjellyzone no, search is not implemented yet. But the find members button should take you to the members page when you don't have any prior relations.

@robotjellyzone
Copy link

robotjellyzone commented Jul 10, 2020

@robotjellyzone no, search is not implemented yet. But the find members button should take you to the members page when you don't have any prior relations.

as you can see @techno-disaster in the provided gif that on clicking Find Members , its not taking anywhere? and it should as you say so, may be your pr is behind the parent repo 🤔

@techno-disaster
Copy link
Contributor Author

@robotjellyzone no, search is not implemented yet. But the find members button should take you to the members page when you don't have any prior relations.

as you can see @techno-disaster in the provided gif that on clicking Find Members , its not taking anywhere? and it should as you say so, may be your pr is behind the parent repo thinking

@robotjellyzone I just checked and this is due to the #108 PR ig, it changes how we do bottom navy. But the fix is not related to this PR. You can approve this and I will create a new issue for this :)

@techno-disaster
Copy link
Contributor Author

Issue up #113

robotjellyzone
robotjellyzone previously approved these changes Jul 20, 2020
Copy link

@robotjellyzone robotjellyzone left a comment

Choose a reason for hiding this comment

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

Ok @techno-disaster i am approving so, that you can create a new issue for that find members issue :)

@techno-disaster techno-disaster mentioned this pull request Jul 27, 2020
6 tasks
@robotjellyzone robotjellyzone added Status: Needs Review PR needs an additional review or a maintainer's review. and removed Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Jul 28, 2020
@techno-disaster
Copy link
Contributor Author

@anitab-org/mentorship-flutter-maintainers can you review this asap?

@isabelcosta
Copy link
Member

@bartekpacia can you please review this?

Copy link
Contributor

@bartekpacia bartekpacia left a comment

Choose a reason for hiding this comment

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

LGTM!

@techno-disaster techno-disaster merged commit 40c5f27 into anitab-org:develop Aug 5, 2020
@techno-disaster techno-disaster deleted the actions-update branch August 5, 2020 00:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Category: Quality Assurance Changes to code or files that improve testing or fixes bugs. Status: Needs Review PR needs an additional review or a maintainer's review. Type: Maintenance Repository maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants