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

Publish cli crate #2946

Merged
merged 2 commits into from
Sep 15, 2019
Merged

Publish cli crate #2946

merged 2 commits into from
Sep 15, 2019

Conversation

ry
Copy link
Member

@ry ry commented Sep 14, 2019

  • Combines cli_snapshots and js into one directory.
  • Extracts TS version at compile time rather than runtime
  • Makes publish work on deno_cli, deno_cli_snapshots, and deno_typescript
  • bumps version awkwardly. sorry.
  • adds new git submodule deno_typescript/typescript
  • improves lint.py and format.py scripts

Try: cargo install deno_cli

Towards #2942

website/manual.md Outdated Show resolved Hide resolved
cli/version.rs Show resolved Hide resolved
js/ts_global.d.ts Show resolved Hide resolved
js/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

tools/lint.py Outdated Show resolved Hide resolved
tools/util.py Outdated Show resolved Hide resolved
@piscisaureus
Copy link
Member

  • Makes publish work on deno_cli, deno_cli_snapshots, and deno_typescript

I don't see any updates to cargo_package.py. Am I missing something?

- Fixes cargo publish on deno_typescript, deno_cli_snapshots, and
  deno_cli.
- Combines cli_snapshots and js into one directory.
- Extracts TS version at compile time rather than runtime
- Bumps version awkwardly - it was necessary to test end-to-end
  publishing. Sorry.
- Adds git submodule deno_typescript/typescript
@ry
Copy link
Member Author

ry commented Sep 15, 2019

I don't see any updates to cargo_package.py. Am I missing something?

So, the other crates (deno_cli, deno_typescript, deno_cli_snapshots) are able to be published with just "cargo publish". I think we should leave to cargo_package.py as it is, because it's doing a particular hack... I could add tools/cargo_publish_others.py (or something) that simply ran cargo publish in the various directories? But I think there are more clean ups to happen still in this area (in particular i want to still rename the crates so we can have 'cargo install deno' work), and I would just as well put that off for a while.

@piscisaureus
Copy link
Member

So, the other crates (deno_cli, deno_typescript, deno_cli_snapshots) are able to be published with just "cargo publish". I think we should leave to cargo_package.py as it is, because it's doing a particular hack... I could add tools/cargo_publish_others.py (or something) that simply ran cargo publish in the various directories? But I think there are more clean ups to happen still in this area, and I would just as well put that off for a while.

Ok, that's great. I was mainly looking for it to understand what the release procedure looks like.
Which crates have to be published, in which order? What command do I run?

Copy link
Member

@piscisaureus piscisaureus left a comment

Choose a reason for hiding this comment

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

I don't fully understand what the release procedure is after this, maybe add some explanation (see #2946 (comment)).

But it seems to work so go ahead and land.

@ry
Copy link
Member Author

ry commented Sep 15, 2019

I've added tools/cargo_publish_others.py which shows the order they have to be run.

We should probably also be running this with the --dry-run argument in CI, because "cargo publish" seems to be different than "cargo build". I would leave running it in CI until later tho.

@ry ry merged commit c9ef182 into denoland:master Sep 15, 2019
@ry ry deleted the publish_cli branch September 15, 2019 22:36
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.

None yet

4 participants