Skip to content

Use google-closure-deps instead of depswriter.py. #839

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

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

kjin
Copy link
Contributor

@kjin kjin commented Apr 26, 2021

In Closure Library, we will be removing depswriter.py and other Python-based build scripts soon. depswriter.py is superceded by google-closure-deps, a similar tool we maintain that is implemented in Node.js.

Here, I've substituted the depswriter.py command with a corresponding command from google-closure-deps; the CLI flags are similar. The difference in output is minimal (see below).

Explanation of "extra" flags:

  • --closure-path="node_modules/google-closure-library/closure/goog" Path to Closure (now required)
  • --file="node_modules/google-closure-library/closure/goog/deps.js" Use the deps.js file bundled with Closure as part of deps calculation
  • --exclude=... Not strictly necessary, but prevents these files from being written to deps.js as trivial, 0-dependency entries

For reference, the diff of the resulting deps.js from running npm run generate-test-files (with lines sorted and formatted for diff clarity) is here.

kjin added a commit to kjin/firebaseui-web that referenced this pull request Apr 26, 2021
kjin added a commit to kjin/firebaseui-web that referenced this pull request Apr 26, 2021
The latter is deprecated and will be deleted soon.
Copy link
Contributor

@bojeil-google bojeil-google left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @kjin. While, testing your change, I discovered some broken tests unrelated to this PR. I will merge this and follow up with the fixes.

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.

2 participants