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

Replacing nodejs-repo-tools with synthtool #2868

Closed
jkwlui opened this issue Sep 28, 2018 · 9 comments
Closed

Replacing nodejs-repo-tools with synthtool #2868

jkwlui opened this issue Sep 28, 2018 · 9 comments
Assignees
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: cleanup An internal cleanup or hygiene concern.

Comments

@jkwlui
Copy link
Member

jkwlui commented Sep 28, 2018

While most of the automated workflows for Node.js client libraries have mostly moved to synthtool, we still use the legacy @google-cloud/repo-tools package for dynamically generating README.md and samples/README.md based on what we have in the samples/ directory.

See nodejs-vision's samples/README.md

@jkwlui jkwlui changed the title Replacing nodejs-repo-tools Replacing nodejs-repo-tools with synthtool Sep 28, 2018
@jkwlui jkwlui self-assigned this Sep 28, 2018
@JustinBeckwith
Copy link
Contributor

Seems like this is something we would want for other languages too, no? Should we consider adding this to synthtool? @theacodes @busunkim96

@theacodes
Copy link

Yes, but let's start with Node.js.

@JustinBeckwith JustinBeckwith added triage me I really want to be triaged. type: cleanup An internal cleanup or hygiene concern. and removed triage me I really want to be triaged. labels Sep 29, 2018
@JustinBeckwith
Copy link
Contributor

Context for others - repo-tools does a lot of things, and we've been slowly moving away towards single purpose tools. Maintaining it has been hard, and at this stage the only thing yoshi is using it for is generating READMEs that embed samples. It's causing hella trouble.

So here's what I think we should do. I want to do this slow like, and responsibly:

  • Let's create a vnext branch in GoogleCloudPlatform/nodejs-repo-tools
  • Essentially delete all the code in that branch and start from scratch
  • Just have a very simple CLI that only does the README.md and samples/README.md manipulation
  • Release this under a different npm dist-tag, so we don't hurt other sample repos still possibly using repo-tools
  • If we decide we like this approach, we split this code out into a new repo, googleapis/readme-generator
  • We try to make the readme generator work across all languages. No one off tools for just nodejs if it can at all be avoided

Looking for input on the approach here @broady @bcoe @crwilcox @busunkim96 @fhinkel ❤️

@JustinBeckwith JustinBeckwith added the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Mar 19, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 19, 2019
@fhinkel
Copy link
Contributor

fhinkel commented Mar 22, 2019

nodejs-docs-samples and nodejs-getting-started is still using many of the test commands and things like repo-tools.getRequest() in the test code.

@JustinBeckwith
Copy link
Contributor

Got it. For our purposes - this is the last mile of dropping the dependency on repo-tools in the client libraries. We're not going to like delete the repo or anything :)

@bcoe
Copy link
Contributor

bcoe commented Mar 22, 2019

@fhinkel would definitely be curious to learn about how you're using the repo; wonder if it makes sense in synthtool too, or if we could trim down the features in node-repo-tools.

@JustinBeckwith
Copy link
Contributor

We very much need to sunset that thing.

@bcoe
Copy link
Contributor

bcoe commented Mar 28, 2019

googleapis/synthtool#214 and googleapis/synthtool#207 have pulled README generation into synthtool, which feels way more consistent (and also means our Node.js libraries won't drift way from a baseline over time; we need to fix the general case).

It would be really cool to try this approach with some other languages now that we have a strategy in place!

@fhinkel let's discuss outside of this issue your other use-cases of repo-tools, and decide whether the library can be deprecated entirely or not.

@fhinkel
Copy link
Contributor

fhinkel commented Mar 28, 2019

@bcoe I'm still discovering new ways of how repo-tools is used 🤣

We use it in tests to make request. getRequest() adds some logic based on environment variables. https://github.com/GoogleCloudPlatform/nodejs-getting-started/blob/master/2-structured-data/test/_api-tests.js#L16

And we're using it to run the tests
https://github.com/GoogleCloudPlatform/nodejs-getting-started/blob/master/5-logging/package.json#L27

And I don't even know what that is
https://github.com/GoogleCloudPlatform/nodejs-getting-started/blob/master/5-logging/package.json#L13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. 🚨 This issue needs some love. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

6 participants