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

Scripts fixes to prevent teachers from continuously needing to input their PAT #369

Merged
merged 4 commits into from
Jul 13, 2022

Conversation

rajbos
Copy link
Contributor

@rajbos rajbos commented Jun 9, 2022

Tested with script 2, 4 and 6

There is also an extra test for the OSTYPE in here, so that the check works on Windows with WSL as well

Copy link

@michel-kamp-work michel-kamp-work left a comment

Choose a reason for hiding this comment

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

this is fine

Copy link

@michel-kamp-work michel-kamp-work left a comment

Choose a reason for hiding this comment

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

done

Copy link

@michel-kamp-work michel-kamp-work left a comment

Choose a reason for hiding this comment

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

reviewed all

Copy link

@michel-kamp-work michel-kamp-work left a comment

Choose a reason for hiding this comment

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

fine

Copy link

@michel-kamp-work michel-kamp-work left a comment

Choose a reason for hiding this comment

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

reviewed

Copy link

@Radian-n Radian-n left a comment

Choose a reason for hiding this comment

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

.

Copy link

@timfry-philips timfry-philips left a comment

Choose a reason for hiding this comment

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

Reviewed

@n1k17a
Copy link

n1k17a commented Jul 7, 2022

.

Copy link

@n1k17a n1k17a left a comment

Choose a reason for hiding this comment

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

.

Copy link

@n1k17a n1k17a left a comment

Choose a reason for hiding this comment

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

.

Copy link

@n1k17a n1k17a left a comment

Choose a reason for hiding this comment

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

Reviewed

@ppremk
Copy link
Contributor

ppremk commented Jul 13, 2022

merging on behalf @rajbos

@ppremk ppremk merged commit c8aaa08 into githubtraining:main Jul 13, 2022
Copy link
Contributor

@parkerbxyz parkerbxyz left a comment

Choose a reason for hiding this comment

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

I just noticed a small typo that can be addressed in a future PR.

@@ -269,8 +269,8 @@ create_issue() {

echo -n "Creating issue: $title... "

# Wait for 1 second to avoid secondary rate limiting
sleep 1
# Wait for 2 second to avoid secondary rate limiting, 1 second was nog enough
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Wait for 2 second to avoid secondary rate limiting, 1 second was nog enough
# Wait for 2 second to avoid secondary rate limiting (1 second was not enough)

@@ -294,8 +294,8 @@ create_pull_request() {

echo -n "Creating pull request: $title... "

# Wait for 1 second to avoid secondary rate limiting
sleep 1
# Wait for 2 seconds to avoid secondary rate limiting, 1 second was nog enough
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Wait for 2 seconds to avoid secondary rate limiting, 1 second was nog enough
# Wait for 2 seconds to avoid secondary rate limiting (1 second was not enough)

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.

7 participants