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

more toolstate comments #69693

Closed
wants to merge 1 commit into from
Closed

more toolstate comments #69693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/bootstrap/toolstate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,11 @@ fn prepare_toolstate_config(token: &str) {
Err(_) => false,
};
if !success {
panic!("git config key={} value={} successful (status: {:?})", key, value, status);
panic!("git config key={} value={} failed (status: {:?})", key, value, status);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was just a typo, wasn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that, and switched one of them in my PR to "unsuccessful". My only thought is that maybe it was written like how some people use expect() and put the expected result/behavior in the string (like expect("does not fail")) instead of explaining the error (expect("error")). I find that quite confusing myself, but I see how reading "expect error" in English is also confusing, since it does the opposite. I kinda wish a different word than expect was chosen. (Sorry, off-topic ramble.)

Copy link
Member Author

Choose a reason for hiding this comment

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

unsuccessful seems fine, too (it was used above so it is more consistent).

}
}

// If changing anything here, then please check that src/ci/publish_toolstate.sh is up to date
// If changing anything here, then please check that `src/ci/publish_toolstate.sh` is up to date
// as well.
git_config("user.email", "7378925+rust-toolstate-update@users.noreply.github.com");
git_config("user.name", "Rust Toolstate Update");
Expand Down Expand Up @@ -379,7 +379,9 @@ fn commit_toolstate_change(current_toolstate: &ToolstateData, in_beta_week: bool
let message = format!("({} CI update)", OS.expect("linux/windows only"));
let mut success = false;
for _ in 1..=5 {
// Update the toolstate results (the new commit-to-toolstate mapping) in the toolstate repo.
// Upload the test results (the new commit-to-toolstate mapping) to the toolstate repo.
// This does *not* change the "current toolstate"; that only happens post-landing
// via `src/ci/docker/publish_toolstate.sh`.
change_toolstate(&current_toolstate, &old_toolstate, in_beta_week);

// `git commit` failing means nothing to commit.
Expand Down
4 changes: 3 additions & 1 deletion src/ci/publish_toolstate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ GIT_COMMIT_MSG="$(git log --format=%s -n1 HEAD)"
cd rust-toolstate
FAILURE=1
for RETRY_COUNT in 1 2 3 4 5; do
# The purpose is to publish the new "current" toolstate in the toolstate repo.
# The purpose of this is to publish the new "current" toolstate in the toolstate repo.
# This happens post-landing, on master.
# (Publishing the per-commit test results happens pre-landing in src/bootstrap/toolstate.rs).
"$(ciCheckoutPath)/src/tools/publish_toolstate.py" "$GIT_COMMIT" \
"$GIT_COMMIT_MSG" \
"$MESSAGE_FILE" \
Expand Down
9 changes: 4 additions & 5 deletions src/tools/publish_toolstate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-

# This script publishes the new "current" toolstate in the toolstate repo (not to be
# confused with publishing the test results, which happens in
# `src/ci/docker/x86_64-gnu-tools/checktools.sh`).
# It is set as callback for `src/ci/docker/x86_64-gnu-tools/repo.sh` by the CI scripts
# when a new commit lands on `master` (i.e., after it passed all checks on `auto`).
# This script computes the new "current" toolstate for the toolstate repo (not to be
# confused with publishing the test results, which happens in `src/bootstrap/toolstate.rs`).
# It gets called from `src/ci/publish_toolstate.sh` when a new commit lands on `master`
# (i.e., after it passed all checks on `auto`).

from __future__ import print_function

Expand Down