Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Add missing submodule command to setup for packages repo. #6276

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

gspencergoog
Copy link
Contributor

When trying to execute the instructions for the packages repo, it kept failing with:

Resolving dependencies...
Because adaptive_scaffold_example depends on code_excerpter from path which doesn't exist (could not find package code_excerpter at "../../../site-shared/packages/code_excerpter"), version solving failed.

This was because the submodule wasn't initialized, so the dependencies weren't available.

@flutter-dashboard flutter-dashboard bot changed the base branch from master_archive to main August 15, 2022 22:23
@flutter-dashboard
Copy link

This pull request was opened against a branch other than main. Since Flutter pull requests should not normally be opened against branches other than main, I have changed the base to main. If this was intended, you may modify the base back to master_archive. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than main, unless this is an intentional hotfix/cherrypick.

@gspencergoog gspencergoog requested review from christopherfujino and stuartmorgan and removed request for christopherfujino August 15, 2022 22:23
@gspencergoog
Copy link
Contributor Author

@stuartmorgan Do you know what's up with this check? Seems like a configuration problem.

Exception: master_archive is not enabled for this .ci.yaml.
Add it to run tests against this PR.

@stuartmorgan
Copy link
Contributor

This is already documented for the specific command: https://github.com/flutter/plugins/blob/main/script/tool/README.md#update-readmemd-from-example-sources

I prefer it there to the main section, since many people will never need to run this particular command.

What instructions in particular were you following? We can add better cross-referencing. We could also update the tool to check for the submodule and give a clear error when it's missing.

@stuartmorgan
Copy link
Contributor

@stuartmorgan Do you know what's up with this check? Seems like a configuration problem.

Exception: master_archive is not enabled for this .ci.yaml.
Add it to run tests against this PR.

That sounds like an infra setup issue; I haven't seen it before.

@gspencergoog
Copy link
Contributor Author

gspencergoog commented Aug 15, 2022

Well, I was using this document to see that I had things set up correctly.

I also looked at all of these, none of which reference submodule updating when they talk about the tools:

I'm happy to add it to any of those instead, depending on where you want it.

@gspencergoog
Copy link
Contributor Author

It occurs to me that we probably have too many places where these instructions exist: it makes it hard to maintain them.

It should probably go here, I think: https://github.com/flutter/flutter/wiki/Setting-up-the-Packages-development-environment

@stuartmorgan
Copy link
Contributor

This should definitely link to https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages (which discusses excerpting, among other things). The flutter/plugins version has it, this was just an oversight on my part.

We could add it here if we make it clear that it's only for the one command. We should probably combine that with the plugins version though so we don't end up with two copies of everything.

I deliberately keep contributing guides out of the main READMEs to have fewer possible places for things to be

@stuartmorgan
Copy link
Contributor

We could add it here if we make it clear that it's only for the one command. We should probably combine that with the plugins version though so we don't end up with two copies of everything.

Actually, it looks like I deliberately pared that down to the very-repo-specific bits and put shared things elsewhere (the other wiki page, the tool README) so I don't think we should add it there.

@stuartmorgan
Copy link
Contributor

Thinking about this more: you were already on the page that had the right instructions, you just didn't find the right part. So while we should add the wiki link I mentioned above, that wouldn't have helped you.

How about we add a new section to ## Getting started called ### Extra setup that mentions that update-excerpts has additional setup and links down to those existing instructions?

@gspencergoog
Copy link
Contributor Author

How about we add a new section to ## Getting started called ### Extra setup that mentions that update-excerpts has additional setup and links down to those existing instructions?

Works for me.

@gspencergoog
Copy link
Contributor Author

This should definitely link to flutter/flutter/wiki/Contributing-to-Plugins-and-Packages (which discusses excerpting, among other things).
The flutter/plugins version has it, this was just an oversight on my part.

I think this was the thing that made it hard for me and for the interns to find it, because without that, the documentation wasn't connected to the packages repo.

@stuartmorgan
Copy link
Contributor

This should definitely link to flutter/flutter/wiki/Contributing-to-Plugins-and-Packages (which discusses excerpting, among other things).
The flutter/plugins version has it, this was just an oversight on my part.

I think this was the thing that made it hard for me and for the interns to find it, because without that, the documentation wasn't connected to the packages repo.

Sent flutter/packages#2476 for this part.

When trying to execute the instructions for the packages repo, it kept failing with:

```
Resolving dependencies...
Because adaptive_scaffold_example depends on code_excerpter from path which doesn't exist (could not find package code_excerpter at "../../../site-shared/packages/code_excerpter"), version solving failed.
```

This was because the submodule wasn't initialized, so the dependencies weren't available.
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
2 participants