Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: app version text bug #254

Closed
wants to merge 1 commit into from
Closed

fix: app version text bug #254

wants to merge 1 commit into from

Conversation

Zfinix
Copy link
Contributor

@Zfinix Zfinix commented Apr 13, 2022

This fixes an import bug where AppVersionText is imported from two locations, package:cosmos_ui_components/components/app_version_text.dart was preferred in this instance.

@Zfinix Zfinix self-assigned this Apr 13, 2022
@Zfinix Zfinix changed the title fix app version text bug fix: app version text bug Apr 13, 2022
@Zfinix Zfinix added the bug 🐛 Something isn't working label Apr 13, 2022
@Zfinix Zfinix requested review from andrzejchm and wal33d006 and removed request for andrzejchm April 13, 2022 03:01
@@ -97,7 +97,7 @@ packages:
description:
path: "packages/cosmos_utils"
ref: main
resolved-ref: "6d6a66c11aec98326728f99eab2672a88a9de8a3"
resolved-ref: "7e2bb662a74772ada30870e605ce3aeae6dfb3cb"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have updated pubspec.lock files for all the packages?

@Zfinix
Copy link
Contributor Author

Zfinix commented Apr 13, 2022 via email

@wal33d006
Copy link
Contributor

The last I remember, it doesn't get git ignored. I think this is happening because pub upgrade might have been run in each of these libraries, can you try reverting the lock files for all except for the template?

@Zfinix
Copy link
Contributor Author

Zfinix commented Apr 13, 2022 via email

@Zfinix
Copy link
Contributor Author

Zfinix commented Apr 13, 2022

The last I remember, it doesn't get git ignored. I think this is happening because pub upgrade might have been run in each of these libraries, can you try reverting the lock files for all except for the template?

I don't see how updating the pub versions are a bad idea as we depend on the latest versions of these packages...

@wal33d006
Copy link
Contributor

wal33d006 commented Apr 14, 2022

The pub versions aren't a bad idea, it's this thing that we don't change things in multiple packages at once, since these packages are being consumed itself by the template, we usually FIRST change things in the packages with a separate PR and then we change the consumers of these packages just to make sure the code within the packages and inside the consumers is good with the latest versions
@andrzejchm What do you say about this?

@Zfinix
Copy link
Contributor Author

Zfinix commented Apr 14, 2022

The pub versions aren't a bad idea, it's this thing that we don't change things in multiple packages at once, since these packages are being consumed itself by the template, we usually FIRST change things in the packages with a separate PR and then we change the consumers of these packages just to make sure the code within the packages and inside the consumers is good with the latest versions @andrzejchm What do you say about this?

But i believe that if these refs are updated using pub update it means that they have been merged into our main branch hence no conflict can occur

@wal33d006
Copy link
Contributor

Exactly my point, the changes should not be shown if they have been already in main

@andrzejchm
Copy link
Contributor

the problem with pubspec.lock changing so often and in multiple packages is because of the fact that all packages lay down in the same repo, so as soon as we change anything anywhere in the repo, running pub get in all the packages will cause the change of the commit ref in pubspec.lock. This is actually fine to always update it in all packages as part f the pr, because if it would break anything, then a build would not pass on the CI. this will no longer be an issue as soon as we switch to separate repos for every package

@Zfinix Zfinix closed this Apr 18, 2022
@Zfinix Zfinix deleted the fix/app-verison-text branch April 18, 2022 08:48
@Zfinix
Copy link
Contributor Author

Zfinix commented Apr 18, 2022

Moved PR to #258

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants