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

Improve domain handling and status checking of updated examples. #5332

Merged
merged 2 commits into from Oct 30, 2017

Conversation

Projects
None yet
4 participants
@adbridge
Contributor

adbridge commented Oct 17, 2017

All the examples cloned from Mercurial should use the new
os.mbed.com domain. Thus update corrects that.

A new option has been added to the update.py script , -s which shows
the status of any PRs raised against the examples that are tagged with
the current release label.

Add mbed-cloud-client-example-internal to examples list.

Improve domain handling and status checking of updated examples.
All the examples cloned from Mercurial should use the new
os.mbed.com domain. Thus update corrects that.

A new option has been added to the update.py script , -s which shows
the status of any PRs raised against the examples that are tagged with
the current release label.
@0xc0170

This should be 3 commits as the description says (3 items that are not related).

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 18, 2017

@adbridge

This comment has been minimized.

Contributor

adbridge commented Oct 18, 2017

Could have been 3 commits but the changes are so small it didn't seem worth it....

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 19, 2017

@theotherjimmy please review

@@ -102,6 +102,19 @@
"auto-update" : true
},
{
"name": "mbed-cloud-client-example-internal",

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 19, 2017

Contributor

While I'm sure we want to test this, I'm sure they don't want it publicly advertised.

This comment has been minimized.

@adbridge

adbridge Oct 24, 2017

Contributor

The client team asked for it to be added.....

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 24, 2017

Contributor

Okay!

try:
repo = github.get_repo(repo_name, False)
except Exception as exc:

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 19, 2017

Contributor

Can you not cache C-c here? Another way: Could you specify the exact exception, or exception list, that would occur? (I know it can be hard, untracked exceptions have a way of being intractable.)

This comment has been minimized.

@adbridge

adbridge Oct 24, 2017

Contributor

As far as I know there is no specific documentation on how the pygithub module returns the github exceptions. I've had a lot of issues in the past trying to narrow down a list and TBH in this case if any exception comes back at all then it's basically fatal...

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 24, 2017

Contributor

Sounds good. I'm generally trying to promote defensive programming because python's "unchecked exceptions" error handling is beginning to drive me crazy.

This comment has been minimized.

@adbridge

adbridge Oct 26, 2017

Contributor

I agree, we could definitely do with more defensive programming !

sys.exit(1)
# Create the full repository filter component
org_str = ''.join(['repo:', 'ARMmbed/', example['name']])

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 19, 2017

Contributor

Why are `'repo:' and 'ARMmbed/' separate strings here? They are just going to be concatenated anyway.

This comment has been minimized.

@adbridge

adbridge Oct 24, 2017

Contributor

yeah , very minor but I could fix it...

@adbridge

This comment has been minimized.

Contributor

adbridge commented Oct 24, 2017

@theotherjimmy please re-review

@theotherjimmy

One more question.

filt = ' '.join([org_str, 'is:pr', tag])
merged = False
issues = github.search_issues(query=(filt))

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 24, 2017

Contributor

Are the () around filt needed? If so, why?

This comment has been minimized.

@adbridge

adbridge Oct 26, 2017

Contributor

It's the pygithub syntax for that command...

This comment has been minimized.

@theotherjimmy

theotherjimmy Oct 26, 2017

Contributor

Thanks!

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Oct 26, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 26, 2017

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 26, 2017

Build : SUCCESS

Build number : 347
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5332/

Triggering tests

/morph test
/morph uvisor-test

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Oct 27, 2017

@theotherjimmy theotherjimmy merged commit acb384c into ARMmbed:master Oct 30, 2017

5 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment