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

fix: do not load cache files when recompile flag is set #1695

Merged
merged 4 commits into from
Feb 7, 2019

Conversation

caijw
Copy link
Contributor

@caijw caijw commented Feb 6, 2019

Fix issue #1637 which is labeled bug.
if recompile flag is set, not load cache files,
else load cache files.
And add a test case test_code_fetch_1.

@kitsonk
Copy link
Contributor

kitsonk commented Feb 6, 2019

LGTM, feels like a good clean solution, but I am not the best person for having an opinion on Rust code.

src/deno_dir.rs Outdated
maybe_source_map_filename: output_source_map_filename
.to_str()
.map(|s| s.to_string()),
maybe_source_map: maybe_source_map,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could be simplified as

Ok(CodeFetchOutput {
  // ...
  maybe_source_code,
  maybe_source_map,
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done ~

src/deno_dir.rs Outdated
maybe_output_code: maybe_output_code,
maybe_source_map_filename: output_source_map_filename
.to_str()
.map(|s| s.to_string()),
Copy link
Member

Choose a reason for hiding this comment

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

nit: could be simplified as map(String::from) (here and elsewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done ~

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.

Thank you - nice fix - LGTM

@ry ry merged commit 37b0574 into denoland:master Feb 7, 2019
ry added a commit to ry/deno that referenced this pull request Feb 9, 2019
- Add deps to --info output (denoland#1720)
- Add --allow-read (denoland#1689)
- Add deno.isTTY() (denoland#1622)
- Add emojis to permission prompts (denoland#1684)
- Add basic WebAssembly support (denoland#1677)
- Add `NO_COLOR` support https://no-color.org/ (denoland#1716)
- Add color exceptions (denoland#1698)
- Fix: do not load cache files when recompile flag is set (denoland#1695)
- Upgrade V8 to 7.4.98 (denoland#1640)
ry added a commit that referenced this pull request Feb 9, 2019
- Add deps to --info output (#1720)
- Add --allow-read (#1689)
- Add deno.isTTY() (#1622)
- Add emojis to permission prompts (#1684)
- Add basic WebAssembly support (#1677)
- Add `NO_COLOR` support https://no-color.org/ (#1716)
- Add color exceptions (#1698)
- Fix: do not load cache files when recompile flag is set (#1695)
- Upgrade V8 to 7.4.98 (#1640)
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