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: automate publishing to pub.dev #719

Merged

Conversation

fischerscode
Copy link
Contributor

@fischerscode fischerscode commented Apr 29, 2022

New Pull Request Checklist

Issue Description

Related issue: #668

Approach

This PR approaches Nr.1 of this commit.
I've decided:

  • to use k-paxian/dart-package-publisher for publishing the packages (has all the features we need)
  • format the code before publishing (pub.dev score)
  • to publish once a release or pre-release has been published on pub.dev
  • use GitHub Actions

Feedback is more then welcome!

TODOs before merging

  • Create PUBDEV_GOOGLE_ACCOUNT_ACCESS_TOKEN and PUBDEV_GOOGLE_ACCOUNT_REFRESH_TOKEN secrets!

- [ ] Add tests
- [ ] Add changes to documentation (guides, repository pages, in-code descriptions)
- [ ] A changelog entry
Maybes:

  • Make sure to only accept pre-release versions when a pre-release is published. (I could write some code for that.)
  • Add some logic, that only publishes, when both packages can be published. (Might add complexity, since the flutter one depends on the dart one.)
  • Add some documentation on the main README

Edit: Since testing this is almost impossible, it's untested.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Apr 29, 2022

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@fischerscode fischerscode added the type:ci CI related issue label Apr 29, 2022
@parse-github-assistant
Copy link

The label type:ci cannot be used here.

@parse-github-assistant parse-github-assistant bot removed the type:ci CI related issue label Apr 29, 2022
@fischerscode
Copy link
Contributor Author

The label type:ci cannot be used here.

Is the label only for issues? Or for PRs created using CI?

@mtrezza
Copy link
Member

mtrezza commented Apr 29, 2022

Yes, most labels are only for issues, as a PR requires an issue reference anyway. In this case there is no label needed as far as I can see.

@mtrezza mtrezza changed the title Automate publishing to pub.dev ci: automate publishing to pub.dev Apr 29, 2022
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks pretty simple, any way we can dry run this to know whether it works? Does the package you're using have a dry run and/or debug option?

@fischerscode
Copy link
Contributor Author

fischerscode commented Apr 29, 2022

@mtrezza, pub has a dry run option, when activated, the command just checks if publishing would be possible. (Credentials are not tested, as far as I know.) The package supports setting it. https://github.com/k-paxian/dart-package-publisher#dryrunonly

So we could set it to true, merge this to master, create a new test branch, update the pubspec.yaml versions in this branch and then create a temporary github release (name/tag does not matter, could be "test"). In the actions tab, we should see the action running and I assume we should see if it should work in the logs. For this, the account should not be needed, we should be able to set the secrets to dummy values.

Some additional information:

  • The action does currently not verify the versions in the pubspec.yaml, so the tag of the release does not have to be the same as the version in the code. I think it would be good to add some logic (later).
  • The action should not fail if the version has already been published (republishing a existing version is not supported by pub.dev) at first I thought that's bad, but on the other side it allows to recreate the github release in case publishing the flutter package failed.

Edit: Should I push a commit that sets dryRunOnly to true?

@fischerscode
Copy link
Contributor Author

Dont mind travis failing, it's the example app using deprecated stuff. We should fix it, but it's no priority.

@mtrezza
Copy link
Member

mtrezza commented Apr 29, 2022

Edit: Should I push a commit that sets dryRunOnly to true?

Yes please.

Could we then simply try it out on the master branch and see what happens, without creating any new branches? Worst case is probably it doesn't get published and we need to delete the release + tag on GitHub and re-create.

@RodrigoSMarques if we merge, it's important that you doesn't also publish manually. Could you please confirm that you are following this thread so we are all on the same page for publishing the next version?

@fischerscode
Copy link
Contributor Author

@mtrezza I've pushed the commit.

Version 3.1.0 is "already" published to pub.dev. So if we create a github commit from the current master branch, the action should realize there is nothing to push and not run dart pub publish --dry-run. However it might be an interesting first test, since nothing can go wrong and we would see if the expected behavior occurs.

You asked @RodrigoSMarques to not publish right now. Is it planned to publish the development branch?

Since dryRunOnly is set, we should not be able to screw something up while testing.
When creating a release where we expect the action not to publish, it might be best to create a pre-release at GitHub.

@RodrigoSMarques
Copy link
Contributor

Version 3.1.0 is "already" published to pub.dev. So if we create a github commit from the current master branch, the action should realize there is nothing to push and not run dart pub publish --dry-run. However it might be an interesting first test, since nothing can go wrong and we would see if the expected behavior occurs.

You asked @RodrigoSMarques to not publish right now. Is it planned to publish the development branch?

Since dryRunOnly is set, we should not be able to screw something up while testing. When creating a release where we expect the action not to publish, it might be best to create a pre-release at GitHub.

I'm updating CHANGELOG.MD and README.MD and will be submitting a PR to the development branch.

I will provide version 3.2.

I'll let you know here as soon as I PR and merge it into the master.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

You asked @RodrigoSMarques to not publish right now. Is it planned to publish the development branch?

What is the purpose of the development branch vs. the master branch? I thought development was for pre-releases, but on GitHub I don't see any pre-releases (alpha / beta), they are all "stable" releases.

@RodrigoSMarques before we merge a PR or create a GH release or a tag, please let's discuss to make sure we are all on the same page.

@fischerscode
Copy link
Contributor Author

@mtrezza
I think there was a best practice that the master branch should contain the latest published (pub.dev) version or something like that.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

To prepare for the next step of release automation, we should use master as the authoritative branch from now on. All other branches will be deleted when we move to release automation and if we need a pre-release branch, we will add a new one according to our Parse Org wide branch model.

@fischerscode
Copy link
Contributor Author

I think that's a good idea. Esp. since new contributors create a PR to master.
Is there documentation regarding the "Parse Org wide branch model".

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

For versioning see CONTRIBUTING.md.

For branch model see parse-community/parse-server#7639 (comment), it's not yet in our docs.

Note that the pre-release branches are optional. We have some repos in which we do not use them either because they are very simple repos or because we expect a high frequency of iterations and want to get developer feedback quickly. For the Parse Flutter SDK I assume we will be fine with only an alpha + release branch.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

Could you rebase this PR onto master? I'm not seeing the button to update the branch, although we have just merged the other PR.

@fischerscode fischerscode force-pushed the automate-publishing-to-pub.dev branch from 9f7afa4 to 6a901d0 Compare April 30, 2022 01:55
@fischerscode
Copy link
Contributor Author

Like this?

@fischerscode
Copy link
Contributor Author

fischerscode commented Apr 30, 2022

pull_request is working!!
But we might have to discuss which combinations we really want to test on :D

Edit: It's getting late, I'll be back tomorrow.

@fischerscode
Copy link
Contributor Author

fischerscode commented Apr 30, 2022

For the Parse Flutter SDK I assume we will be fine with only an alpha + release branch.

👍

Edit: later when automating publishing completely...
We have to keep in mind, that the version in the pubspec.yaml files have to always be adjusted.
Should we let Actions determine the next version (based on PR labels (breaking/major,feat/minor,bug/patch)) or should we setup actions to push once we change the version by pushing?

When pushing to release, the logic is simple, just remove the prerelease part of the versions and publish.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

For manual release, the person manually creates a tag as part of the GitHub release form. In the workflow, the version tag needs to be taken from the release that triggered the workflow; this is a variable that is accessible in the workflow.

For automated release, versioning will be determined by the auto-release framework, we'll have a more detailed discussion about this when we tackle step 2.

@fischerscode
Copy link
Contributor Author

the version tag needs to be taken from the release that triggered the workflow

So we just set the tag as the version in pubspec.yaml? I think it has to be right before already, since the code attached to the github release is otherwise dirty.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

I think so; manually setting the version in pubspec.yaml while we do manual release is part of the "release preparation" PR, in which we also check the changelog entries. This will all go away with auto-release anyway, but it would be good to document the steps how to do manual release, even if we may only have to do it 1 time.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

@fischerscode Do you know how I can find these tokens? Accord to the docs they should be in ~/.pub-cache/credentials.json but how can I get that file created? I assume I have to somehow authenticate through the dart CLI?

@fischerscode
Copy link
Contributor Author

fischerscode commented Apr 30, 2022

I think it should ask you to log in when you run flutter pub publish (in package/dart or package/flutter).
Run flutter pub publish --dry-run first and make sure it tells you the version is already published.

@fischerscode
Copy link
Contributor Author

@mtrezza Just had a look, flutter pub login exists.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

Thanks, that worked! For the record, the credential file location moved to:

~/Library/Application Support/dart/pub-credentials.json

See https://stackoverflow.com/questions/70487479/whats-the-new-location-of-credentials-json-for-dart-flutter-pub

.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
@fischerscode
Copy link
Contributor Author

Thanks, that worked! For the record, the credential file location moved to:

~/Library/Application Support/dart/pub-credentials.json

See https://stackoverflow.com/questions/70487479/whats-the-new-location-of-credentials-json-for-dart-flutter-pub

So Create PUBLISH_OAUTH_ACCESS_TOKEN and PUBLISH_OAUTH_REFRESH_TOKEN secrets! is done?

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

Yes, the secrets exist, with different names though, see my review comment.

@mtrezza
Copy link
Member

mtrezza commented Apr 30, 2022

What happens if the dart publishing succeeds but the dart fails? Is that possible and what would be the consequences / complications?

Should we change the workflow to first do a dry run of both and only if both pass start to actually publish each of them?

@fischerscode
Copy link
Contributor Author

What happens if the dart publishing succeeds but the dart fails? Is that possible and what would be the consequences / complications?

You mean when dart is published and flutter fails?
We could delete the GitHub release, fix what lead to the flutter package not publishing and then recreate the GitHub release.
Dart wont be republished (pub.dev policy) but the action should not fail.
Afterwards the flutter package should be published.

@fischerscode
Copy link
Contributor Author

Should we change the workflow to first do a dry run of both and only if both pass start to actually publish each of them?

We could, should not be that hard.

@fischerscode
Copy link
Contributor Author

Note: Currently dryRunOnly: true is still set.

Copy link
Member

@mtrezza mtrezza 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! I think we can merge this and once we have the other issues fixed just do a test release and see what happens. What do you think?

@fischerscode
Copy link
Contributor Author

Looks good! I think we can merge this and once we have the other issues fixed just do a test release and see what happens. What do you think?

Sounds good!

@mtrezza mtrezza merged commit c9ef6bf into parse-community:master Apr 30, 2022
@mtrezza mtrezza linked an issue Apr 30, 2022 that may be closed by this pull request
7 tasks
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.

Automate publishing process via CI
3 participants