Skip to content

[go_router] Fix go_router_builder example #2177

Closed
johnpryan wants to merge 4 commits intoflutter:mainfrom
johnpryan:fix-builder-example
Closed

[go_router] Fix go_router_builder example #2177
johnpryan wants to merge 4 commits intoflutter:mainfrom
johnpryan:fix-builder-example

Conversation

@johnpryan
Copy link
Contributor

@johnpryan johnpryan commented Jun 1, 2022 •
edited
Loading

If the recursive search doesn't find any sub-routes, the search should continue for other top-level routes.

fixes flutter/flutter#105150

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@johnpryan johnpryan requested a review from kevmoo June 1, 2022 17:02
nilsreichardt
nilsreichardt reviewed Jun 1, 2022
View reviewed changes
Copy link
Contributor

@nilsreichardt nilsreichardt left a comment

Choose a reason for hiding this comment

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

Shouldn't we update the CHANGELOG.md file?

@johnpryan
Copy link
Contributor Author

johnpryan commented Jun 1, 2022

Is it okay that the legacy-version-analyze CHANNEL:2.10.5 and legacy-version-analyze CHANNEL:2.8.1 aren't passing?

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Jun 1, 2022

We should revert the broken PR, rather than try to fix it forward. Per discussion in Discord, there's a fundamental problem with the go_router_builder example setup that needs to be addressed. I'll comment further in the issue.

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Jun 1, 2022

I filed #2183 for the revert and commented in the issue. Once the revert is in we can fix go_router_builder to make sure this doesn't happen again, then reland.

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Jun 1, 2022

Is it okay that the legacy-version-analyze CHANNEL:2.10.5 and legacy-version-analyze CHANNEL:2.8.1 aren't passing?

No, that means that you did something that requires Flutter 3.0 (in this case, assuming non-null bindings) but didn't change the minimum Flutter version for the package to 3.0, meaning that if published it would break people on older versions of Flutter. https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#when-to-update-the-required-version

@johnpryan johnpryan closed this Jun 1, 2022
This was referenced Jun 1, 2022
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kevmoo kevmoo kevmoo approved these changes

+1 more reviewer

@nilsreichardt nilsreichardt nilsreichardt left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

p: go_router_builder p: go_router

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

go_router_builder example is broken

4 participants

@johnpryan @stuartmorgan-g @kevmoo @nilsreichardt