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.

Sign up for GitHub

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

Jump to bottom

Restore old execution log diffing docs #15274

Closed
wants to merge 1 commit into from
Closed

Restore old execution log diffing docs #15274

wants to merge 1 commit into from

Conversation

keith
Copy link
Member

@keith keith commented Apr 16, 2022

While the new json execution log file is handy in some cases, it doesn't
provide the same diffing behavior as the binary file and the execlog
parser tool. The json file contains walltime attributes for each
action that (likely) differ per run, and it's not in any guaranteed
order.

This is a partial revert of b35944e.

@sgowroji sgowroji added the team-Documentation Documentation improvements that cannot be directly linked to other team labels label Apr 16, 2022
@sgowroji sgowroji added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 21, 2022
@coeuvre
Copy link
Member

coeuvre commented Apr 27, 2022

--execution_log_json_file is easier to get started and it is worth mentioning it in the doc. Instead of replacing it, I would suggest add another section for --execution_log when diffing matters.

@keith
Restore old execution log diffing docs
fc071a0 
While the new json execution log file is handy in some cases, it doesn't
provide the same diffing behavior as the binary file and the execlog
parser tool. The json file contains `walltime` attributes for each
action that (likely) differ per run, and it's not in any guaranteed
order.

This is a partial revert of b35944e.
@keith keith force-pushed the ks/restore-old-execution-log-diffing-docs branch from 0563a3b to fc071a0 Compare April 27, 2022 20:41
@keith
Copy link
Member Author

keith commented Apr 27, 2022

Added a sentence on that flag

coeuvre
coeuvre approved these changes Apr 28, 2022
View reviewed changes
Copy link
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Thanks!

@bazel-io bazel-io closed this in f2a7475 Apr 28, 2022
@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coeuvre coeuvre coeuvre approved these changes

@gregestren gregestren Awaiting requested review from gregestren gregestren is a code owner

@fweikert fweikert Awaiting requested review from fweikert fweikert is a code owner

Assignees

@coeuvre coeuvre

Labels
team-Documentation Documentation improvements that cannot be directly linked to other team labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@keith @coeuvre @sgowroji @ShreeM01