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

Fix mbed-cli issue #468. Add LPCTargetCode.lpc_patch to POST_BINARY_WHITELIST #4085

Merged
merged 1 commit into from Apr 6, 2017

Conversation

Projects
None yet
6 participants
@wdwalker
Contributor

wdwalker commented Mar 30, 2017

Notes:

  • Pull requests will not be accepted until the submitter has agreed to the contributer agreement.
  • This is just a template, so feel free to use/remove the unnecessary things

Description

Although the post_binary_hooks are not used by the exporters at this time, the exporters do set up a POST_BINARY_WHITELIST and reject anything with a post_binary_hook not in the whitelist. This prevents LPCTargets from working with exporters. This patch adds the LPCTarget to the whitelist of all exporters.

Status

READY
Resolves ARMmbed/mbed-cli#468

Todos

  • Tests - /morph export-build
@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 30, 2017

I edited the description to remove all the excess.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 30, 2017

/morph export-build

@theotherjimmy theotherjimmy self-requested a review Mar 30, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 30, 2017

The process we go through for merging PRs is something like:

  1. PR submitted
  2. PR reviewed and testing plan decided
  3. Wait for tests to finish
  4. mark Ready For Merge and notify gatekeepers.
@mbed-bot

This comment has been minimized.

mbed-bot commented Mar 30, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph export-build

Output

mbed Build Number: 150

All exports and builds passed!

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 31, 2017

@0xc0170 0xc0170 added the needs: CLA label Mar 31, 2017

@wdwalker

This comment has been minimized.

Contributor

wdwalker commented Mar 31, 2017

I believe so. When I go to https://developer.mbed.org/contributor_agreement/, it says "You accepted the Contributor Agreement on Wed 29 Mar 2017" at the bottom. Many thanks.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 31, 2017

@wdwalker Whats your username? https://developer.mbed.org/users/wdwalker/ gives an error?

@wdwalker

This comment has been minimized.

Contributor

wdwalker commented Mar 31, 2017

D'Oh - it's wwalker in mbed, but wdwalker in GitHub.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 31, 2017

It looks like you pushed the commit that fixes the other issue onto this branch. Could you create a feature branch for that commit and submit it as a separate PR? This will reduce the time it takes to get these PRs merged, and keep the discussions in each PR focused.

@wdwalker

This comment has been minimized.

Contributor

wdwalker commented Mar 31, 2017

Yeah - just saw that. Didn't know that was going to happen. Sorry about that. Thought I could use my fork over and over for different PRs. (I see now that I should create a feature branch on my fork for each different PR, right?)

So, what state did I leave the fork in? Is the PR for this specific issue (#468) now screwed up?

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 31, 2017

So, what state did I leave the fork in? Is the PR for this specific issue (#468) now screwed up?

It's fixable.

You could:

  • create a feature branch from this branch: git checkout -b lpc-pindefs
  • remove the unrelated commits from that feature branch: git rebase -i origin/master
    • delete the lines related to commits that you don't want included
  • checkout the original branch git checkout master
  • remove the unrelated commits from the original feature branch using the same method.

Yeah - just saw that. Didn't know that was going to happen. Sorry about that. Thought I could use my fork over and over for different PRs.

You can, with different branches.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 31, 2017

Nice side effect of this little mistake: you learn more git-foo.

@wdwalker wdwalker force-pushed the wdwalker:master branch to 1324742 Mar 31, 2017

@wdwalker

This comment has been minimized.

Contributor

wdwalker commented Mar 31, 2017

OK - think I fixed it. Sorry for the confusion. At least now I know I can use feature branches on my fork. I like feature branches. (The past few years have been spent in an Atlassian environment and the workflow was slightly different.)

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Mar 31, 2017

It looks like you got it! Thanks!

@0xc0170

0xc0170 approved these changes Apr 3, 2017

@0xc0170 0xc0170 added the needs: CI label Apr 3, 2017

@bridadan bridadan added ready for merge and removed needs: CI labels Apr 4, 2017

@sg- sg- merged commit 14eadf3 into ARMmbed:master Apr 6, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-export-build Job has 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