Skip to content

Conversation

@themisir
Copy link
Contributor

@themisir themisir commented May 3, 2021

Fixes #37

@themisir themisir changed the title Null-safety migration WIP Null-safety migration May 3, 2021
@themisir
Copy link
Contributor Author

themisir commented May 3, 2021

Actually I think nnbd version should be maintained separately on different branch for some time. At least that's what we did when migrating hive to nnbd.

@themisir themisir closed this May 3, 2021
@curquiza curquiza mentioned this pull request May 4, 2021
@themisir themisir reopened this May 4, 2021
@curquiza
Copy link
Member

curquiza commented May 5, 2021

Hello @themisir! Thanks for doing this work :)
Ping me when you have finished this PR 🙂

@themisir themisir changed the title WIP Null-safety migration Null-safety migration May 5, 2021
@themisir
Copy link
Contributor Author

themisir commented May 5, 2021

@curquiza it's ready. Just updated pubspec.yaml version to v0.2.0 and tests are passing (if dart >2.12).

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Thanks a lot @themisir 😄

  • Can you update the bors.toml file please? Some tests have to be removed 😇
  • Does the README need changes? I think about the Getting Started section

@themisir
Copy link
Contributor Author

themisir commented May 5, 2021

  • Can you update the bors.toml file please? Some tests have to be removed 😇

I've updated bors.toml because testing null safety requires dart ^2.12.

Remove nnbd branch from workflow branches

Co-authored-by: Clémentine Urquizar <clementine@meilisearch.com>
@curquiza
Copy link
Member

curquiza commented May 5, 2021

I've updated bors.toml because testing null safety requires dart ^2.12.

My bad, I miss checked! :)

Update dependency version to: `meilisearch: ^0.2.0`
@themisir themisir requested a review from curquiza May 5, 2021 17:00
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Awesome!! 🔥 Thanks @themisir!

@curquiza
Copy link
Member

curquiza commented May 6, 2021

bors merge

@bors
Copy link
Contributor

bors bot commented May 6, 2021

Build succeeded:

@bors bors bot merged commit 519c3a8 into main May 6, 2021
@bors bors bot deleted the nnbd branch May 6, 2021 18:07
@themisir
Copy link
Contributor Author

themisir commented May 6, 2021

@curquiza out of context, but what does bors exactly?

@themisir themisir mentioned this pull request May 6, 2021
@curquiza
Copy link
Member

curquiza commented May 6, 2021

TLDR: Bors is a tool that applies a rebase before running the tests and finally merging if they succeed.

To be more accurate, Bors does not concretely rebase the branch of the PR: it merges the commits of the PR into another branch (staging) that is already up-to-date with main. If the tests do not pass on the staging branch, Bors returns a failure and stops the merge of the PR.

We use it because:

  • we don't have to ask the contributors to rebase anymore which is a really nice point 😁
  • it can handle multiple PRs at the same time: no need to wait for the PR of your colleague to be merged to merge yours.

I wrote a guide about it here: https://github.com/meilisearch/integration-guides/blob/main/guides/bors.md
Also, I wrote an article about our usage of bors, but it's not released yet 😇

Also, bors is a tool widely used (and created) by the rust community

@curquiza curquiza added the breaking-change The related changes are breaking for the users label May 6, 2021
@themisir
Copy link
Contributor Author

themisir commented May 6, 2021

Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change The related changes are breaking for the users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration to null-safety

3 participants