Skip to content

PSA: release.py updates to fixes situation where the script would get stuck #10038

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
Merged
merged 1 commit into from
Mar 12, 2019
Merged

PSA: release.py updates to fixes situation where the script would get stuck #10038

merged 1 commit into from
Mar 12, 2019

Conversation

orenc17
Copy link
Contributor

@orenc17 orenc17 commented Mar 11, 2019

Description

  • Remove stdout redirection.
  • use git diff --quiet

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@adbridge

Release Notes

@adbridge
Copy link
Contributor

@orenc17 Is this essential for 5.12 or should we see if it works first and if so just bring this in for 5.12.1 ?

@orenc17
Copy link
Contributor Author

orenc17 commented Mar 11, 2019 •
edited
Loading

The script seem to get stuck on big git diffs, so yes this is crucial
Sorry for the mess

@adbridge adbridge self-requested a review March 11, 2019 15:35
adbridge
adbridge approved these changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@adbridge adbridge left a comment

Choose a reason for hiding this comment

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

AFAICT looks ok to me

@adbridge
Copy link
Contributor

@orenc17 I assume this latest change has been fully tested ?

@orenc17
Copy link
Contributor Author

orenc17 commented Mar 11, 2019

Yes

@0xc0170 0xc0170 requested a review from a user March 11, 2019 15:44
0xc0170
0xc0170 requested changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Minnor changes fixing the script from getting stuck

Can you add to the commit msg - what errors is this fixing - big diffs or similar you already mentioned.

What use cases are being fixed here is the question to be answered

Minor changes fixing the script from getting stuck
dad75b5 
* git diff of bin/hex files could be huge print and make
* calling python unbuffered with stdout=subprocess.PIPE could be problematic
@orenc17
Copy link
Contributor Author

orenc17 commented Mar 11, 2019

@0xc0170 Fixed commit message

0xc0170
0xc0170 approved these changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

+1 for the commit msg update

@adbridge
Copy link
Contributor

ci started

@mbed-ci
Copy link

mbed-ci commented Mar 11, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 1
Build artifacts

@NirSonnenschein
Copy link
Contributor

@cmonr CI has passed, should be ready to merge pending approval by @ChiefBureaucraticOfficer.

ghost
ghost approved these changes Mar 11, 2019
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Meets criteria, PSA. Approved.

@ghost ghost added the PM_ACCEPTED label Mar 11, 2019
@cmonr cmonr merged commit bf6e6ee into ARMmbed:master Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@adbridge adbridge adbridge approved these changes

@NirSonnenschein NirSonnenschein NirSonnenschein approved these changes

Assignees
No one assigned
Labels
release-version: 5.12.0-rc2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@orenc17 @adbridge @mbed-ci @NirSonnenschein @0xc0170 @cmonr