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

Show correct commit in quarto check install #9526

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Apr 29, 2024

Correctly run git rev-parse in quarto-cli repo.
Otherwise, the commit shown was the one of the project from which quarto check was run, or another if no .git in this project.

Follow up on #9145

Otherwise, the commit shown was the one of the project from which `quarto check` was run, or another if no .git in this project.
});
if (gitHead.stdout) {
info(` commit: ${gitHead.stdout.trim()}`);
if (Deno.env.get("QUARTO_ROOT")) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In other places we do join(quartoConfig.sharePath(), "../..") and do not assume QUARTO_ROOT is set.

Though it seems better to assume it is, at least it should from interactive run of quarto check from a dev version

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And it seems I should do the same here, because QUARTO_ROOT is not set in our every run !

Copy link
Collaborator

Choose a reason for hiding this comment

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

And it seems I should do the same here, because QUARTO_ROOT is not set in our every run !

Do we understand why QUARTO_ROOT is not set?

This reverts commit cf14684.

Just to check if QUARTO_ROOT was unset or just this was a type check issue
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

2 participants