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

[Feature]: Let merge this :) #1

Closed
chipweinberger opened this issue Dec 23, 2024 · 35 comments
Closed

[Feature]: Let merge this :) #1

chipweinberger opened this issue Dec 23, 2024 · 35 comments

Comments

@chipweinberger
Copy link

chipweinberger commented Dec 23, 2024

What is your feature request?

I'm looking it over, and i'm impressed with how it turned out. Thanks for taking the initiative.

@chipweinberger
Copy link
Author

chipweinberger commented Dec 23, 2024

I added you as a collaborator.

Please feel free to take a more active roll in FBP. I've been trying to reduce my time commitment, so it's great to have someone to split it with.

Are you okay to handle the work of merging the remaining commits? We should also probably update the readme.

Then when you are ready, do a "Release 1.35.0" commit & add a git tag.

@chipweinberger
Copy link
Author

chipweinberger commented Dec 23, 2024

I added this setting to pub.dev, so hopefully just by pushing a tag of the format X.X.X, we can publish to pub.dev.

Screenshot 2024-12-23 at 11 16 56 AM

@chipweinberger
Copy link
Author

chipweinberger commented Dec 23, 2024

Also just a bit of context - the last time we talked I was right before product launch, so I very much wanted to avoid active development on the main branch.

But now the launch already happened and the launch bugs are ironed out. So now is a much better time for these developments.

@tnc1997
Copy link
Owner

tnc1997 commented Jan 1, 2025

Hi @chipweinberger, it's great that you're keen to merge these platform interface changes into the upstream repository!

In order to minimise your time commitment would you like me to maintain the Linux and Web platforms separately?

@chipweinberger
Copy link
Author

It's a good question.

I think just all one repository is fine. But up to you!

@tnc1997
Copy link
Owner

tnc1997 commented Jan 2, 2025

I think just all one repository is fine.

That sounds good to me, if you're happy with all platform implementations in the upstream repository, then I will open a Pull Request to merge the changes in this repository.

Have you got any upcoming changes that I should plan to merge into this fork before opening the Pull Request?

@chipweinberger
Copy link
Author

no need for a pr! / you can merge as you please

you could work off master branch too if you want!

@chipweinberger
Copy link
Author

chipweinberger commented Jan 2, 2025

as for upcoming changes, none. But these eventually will be fixed:

chipweinberger#1047

chipweinberger#1059

@tnc1997
Copy link
Owner

tnc1997 commented Jan 3, 2025

Thank you for confirming that, I have prepared a branch that we should be able to merge into upstream's master.

Once merged, we should publish the platform interface, then the implementations, then the main package itself.

Are you okay to handle the work of merging the remaining commits?

I am more than happy to handle the merging of the linked Pull Request into master if you are happy for me to.

I added this setting to pub.dev, so hopefully just by pushing a tag of the format X.X.X, we can publish to pub.dev.

Just to double check, does this take into account that we will have a monorepo, that contains multiple packages?

@chipweinberger
Copy link
Author

Just to double check, does this take into account that we will have a monorepo, that contains multiple packages?

not sure

@tnc1997
Copy link
Owner

tnc1997 commented Jan 5, 2025

I added this setting to pub.dev, so hopefully just by pushing a tag of the format X.X.X, we can publish to pub.dev.

Would you be able to update the tag that is used in the automated publishing settings to handle multiple packages?

If your repository contains multiple packages, give each a separate tag-pattern. Consider using a tag-pattern like my_package_name-v{{version}} for a package named my_package_name.

Based on the guidance a tag pattern of flutter_blue_plus-v{{version}} for the main package would be good to use.

@chipweinberger
Copy link
Author

okay

Screenshot 2025-01-05 at 11 44 09 AM

@tnc1997
Copy link
Owner

tnc1997 commented Jan 5, 2025

Looks good to me, I will merge the associated Pull Request, then set up some GitHub Actions to publish the packages.

Upon reviewing the documentation, the initial version of the newly added packages will need to published manually.

@tnc1997
Copy link
Owner

tnc1997 commented Jan 15, 2025

what is the current state of master branch on flutter_blue_plus?

The master branch has the platform interface and the federated packages ready for reviewing, testing, publishing.

@chipweinberger
Copy link
Author

okay, i pushed all the packages.

@chipweinberger
Copy link
Author

I've also added all these actions. So hopefully you can figure out how to publish easily.

Screenshot 2025-01-14 at 11 40 58 PM

@chipweinberger
Copy link
Author

last, just a small style thing, please push a "Release 1.35.0" commit. Just so we can easily see each version in the commit history.

@chipweinberger
Copy link
Author

Screenshot 2025-01-14 at 11 47 13 PM

@tnc1997
Copy link
Owner

tnc1997 commented Jan 15, 2025

So hopefully you can figure out how to publish easily.

Now that the initial versions have been published we should be able to push git tag(s) to run the relevant workflow(s).

@tnc1997
Copy link
Owner

tnc1997 commented Jan 15, 2025

please push a "Release 1.35.0" commit

Out of interest would you like to differentiate between the different packages (and versions) in the commit message?

@chipweinberger
Copy link
Author

chipweinberger commented Jan 15, 2025

Yes, we probably ought to. I'm not quite sure, but whatever makes the most sense.

e.g. Whenever we update a pubspec.yaml version number

@chipweinberger
Copy link
Author

chipweinberger commented Jan 16, 2025

Okay, I think I know how to do it.

We will use a single version number, essentially.

Whenever we update any package, we'll also update flutter_blue_plus/pubspec.yaml to a new version.

  1. flutter_blue_plus will always be on the latest version
  2. the other packages will either be on the latest version, or a previous version if it did not change
dependencies:
  flutter:
    sdk: flutter
  flutter_blue_plus_android: ^1.35.0
  flutter_blue_plus_ios: ^1.35.0
  flutter_blue_plus_linux: ^1.0.0
  flutter_blue_plus_macos: ^1.35.0
  flutter_blue_plus_platform_interface: ^1.0.0
  flutter_blue_plus_web: ^1.0.0

Also, we should remove the ^ symbol. There's no need to mix and match versions, it just creates unnecessary complexity. For every flutter_blue_plus, it should be grouped with a specific set of versions that it is meant for. People can always fork if they need more control.

A huge advantage is now people can just tell us a single version number, and we'll know exactly what they are using.

Essentially, I am trying to keep the simplicity of non-federated.

Then we can just have a single "Release 1.X.X" commit too.


For Windows, since we don't plan to control it, we'll just use flutter_blue_plus_windows: ^1.0.0

@chipweinberger
Copy link
Author

chipweinberger commented Jan 16, 2025

I appreciate your push back.

Do we actually plan to validate mixing & matching? Is it worth our time to debug issues from mixing & matching? I know for me, I don't have time or motivation to deal with that.

FBP has had a single version number for years, and it's never bothered anyone.

IMO, let's start with one version number. It's simpler for us, and simpler for users.

We can always introduce ranges later.

@tnc1997
Copy link
Owner

tnc1997 commented Jan 16, 2025

IMO, let's start with one version number. It's simpler for us, and simpler for users.

Of course, it is your decision as the lead maintainer, I was simply pointing out what the other Flutter packages do.

@chipweinberger
Copy link
Author

chipweinberger commented Jan 16, 2025

Lead maintainer, for now :)

Thanks again for all your work on platform interface.

@tnc1997
Copy link
Owner

tnc1997 commented Jan 16, 2025

Thanks again for all your work on platform interface.

No problem at all, it is my pleasure!

I am very grateful to be able to use flutter_blue_plus on more platforms.

Thank you very much for your support reviewing, testing, publishing the platform interface and federated packages.

@chipweinberger
Copy link
Author

chipweinberger commented Jan 20, 2025

@ekuleshov, just want to make you aware of this: #1 (comment)

you can push tags and it should release to pub.dev, assuming we setup the github actions correctly.

In terms of version numbers, I'm flexible. I prefer using 1.X.X for the main flutter_blue_plus repo. For the others, I don't really care :)

@chipweinberger
Copy link
Author

chipweinberger commented Jan 20, 2025

@tnc1997 , what is the purpose of this commit? chipweinberger@3b5e4d1

Why do they all need to return bool?

If it's to avoid future breaking changes, we should not care about that. No one else is using the platform interface besides us. We can break it anytime. And we can coordinate with @chan150 about Windows.

IMO, lets document that the platform interface is intended for internal use, and has no stability guarentees. Then we can stop worrying about 'breaking' it.

@chipweinberger
Copy link
Author

Also worth pointing out, the interface will change dramatically when we implement this: chipweinberger#795

@ekuleshov
Copy link

Also worth pointing out, the interface will change dramatically when we implement this: chipweinberger#795

Not necessarily. E.g. if we'd just rely on the order of services/chars in the public api. Then internally could use indexes or use internal subclass of Guid with an index

@chipweinberger
Copy link
Author

chipweinberger commented Jan 20, 2025

sure. let's do whatever makes most sense. I don't care about breaking the platform interface. It just came out. It's not stable yet, and has no public purpose yet.

@tnc1997
Copy link
Owner

tnc1997 commented Jan 20, 2025

Why do they all need to return bool?

After reviewing the Android/iOS/macOS platform implementations, this is the result that those method channels return, so returning a bool aligns the platform interface with these pre-existing federated platform implementations. If you like we could update the Android/iOS/macOS platform implementations as part of this piece of work to not return a bool.

@tnc1997
Copy link
Owner

tnc1997 commented Jan 20, 2025

In terms of version numbers, I'm flexible. I prefer using 1.X.X for the main flutter_blue_plus repo.

That sounds good, would you be comfortable for us to use semantic versioning for flutter_blue_plus_android flutter_blue_plus_darwin flutter_blue_plus_linux flutter_blue_plus_platform_interface flutter_blue_plus_web, but continue to use 1.X.X for the main flutter_blue_plus package that developers would add as a dependency?

@chipweinberger
Copy link
Author

Why do they all need to return bool?

After reviewing the Android/iOS/macOS platform implementations, this is the result that those method channels return, so returning a bool aligns the platform interface with these pre-existing federated platform implementations. If you like we could update the Android/iOS/macOS platform implementations as part of this piece of work to not return a bool.

oh ok. Yes, good point. I can't recall if we need to return anything for those, or if it just happened to be like that.

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

No branches or pull requests

3 participants