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

feat(std) support AbortSignal in writeFile #11568

Merged
merged 5 commits into from
Aug 6, 2021

Conversation

benjamingr
Copy link
Contributor

Refs #10181

This continues the work in #10943 and adds AbortSignal support to writeFile (and since it uses the same infrastructure writeTextFile)

cc @lucacasonato and @ry who reviewed the last one :)

@benjamingr
Copy link
Contributor Author

A little help would be appreciated here :)

I ran lint and format and tests pass:

➜  deno git:(writefile-abortsignal) ./target/debug/deno --unstable run --allow-all ./tools/lint.js  
dlint
clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.38s
➜  deno git:(writefile-abortsignal) ./target/debug/deno --unstable run --allow-all ./tools/format.js 
dprint
rustfmt 186 file(s)

Cargo test passes - can anyone help me with the failure? The only git diff is:

➜  deno git:(writefile-abortsignal) git status                                                
On branch writefile-abortsignal
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)
	modified:   test_util/std (new commits, modified content)
	modified:   test_util/wpt (new commits)

no changes added to commit (use "git add" and/or "git commit -a")

Am I supposed to commit that or something?

@caspervonb
Copy link
Contributor

caspervonb commented Aug 3, 2021

A little help would be appreciated here :)

I ran lint and format and tests pass:

➜  deno git:(writefile-abortsignal) ./target/debug/deno --unstable run --allow-all ./tools/lint.js  
dlint
clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.38s
➜  deno git:(writefile-abortsignal) ./target/debug/deno --unstable run --allow-all ./tools/format.js 
dprint
rustfmt 186 file(s)

Cargo test passes - can anyone help me with the failure? The only git diff is:

➜  deno git:(writefile-abortsignal) git status                                                
On branch writefile-abortsignal
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)
	modified:   test_util/std (new commits, modified content)
	modified:   test_util/wpt (new commits)

no changes added to commit (use "git add" and/or "git commit -a")

Am I supposed to commit that or something?

Pay no attention to the submodule bumps (unless you need to bump it for a reason).

Try running:

deno lint cli/tests/unit

@benjamingr
Copy link
Contributor Author

@caspervonb thanks, that helped - would it help if I made a PR to automatically do that when you run tools/lint.js ?

@caspervonb
Copy link
Contributor

@caspervonb thanks, that helped - would it help if I made a PR to automatically do that when you run tools/lint.js ?

As far as I recall, we originally set it up that way so that we wouldn't have to build cli to lint.
Gonna defer to @bartlomieju

@benjamingr
Copy link
Contributor Author

This should be ready for review :)

@bartlomieju
Copy link
Member

bartlomieju commented Aug 3, 2021

A little help would be appreciated here :)

I ran lint and format and tests pass:

➜  deno git:(writefile-abortsignal) ./target/debug/deno --unstable run --allow-all ./tools/lint.js  
dlint
clippy
    Finished dev [unoptimized + debuginfo] target(s) in 0.38s
➜  deno git:(writefile-abortsignal) ./target/debug/deno --unstable run --allow-all ./tools/format.js 
dprint
rustfmt 186 file(s)

Cargo test passes - can anyone help me with the failure? The only git diff is:

➜  deno git:(writefile-abortsignal) git status                                                
On branch writefile-abortsignal
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)
	modified:   test_util/std (new commits, modified content)
	modified:   test_util/wpt (new commits)

no changes added to commit (use "git add" and/or "git commit -a")

Am I supposed to commit that or something?

git submodule update --init --recursive

@caspervonb thanks, that helped - would it help if I made a PR to automatically do that when you run tools/lint.js ?

I don't think git submodules should be mixed with linting of JS and Rust code.

deno lint cli/tests/unit

This is not a valid command, we don't use deno lint for linting internal code; we only do that via tools/lint.js script.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM, no comments. I will let Luca review before landing

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM- thanks @benjamingr

@ry ry merged commit 2b53602 into denoland:main Aug 6, 2021
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