Skip to content

Fix #10589 for STM32F4 #10595

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

Conversation

embeddedteam103
Copy link
Contributor

Description

Resolve #10589 for STM32F4 targets.

Pull request type

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

Reviewers

Release Notes

@ciarmcom ciarmcom requested review from a team May 16, 2019 13:01
@ciarmcom
Copy link
Member

@embeddedteam103, thank you for your changes.
@ARMmbed/mbed-os-core @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-test @ARMmbed/mbed-os-crypto @ARMmbed/mbed-os-maintainers please review.

@SeppoTakalo
Copy link
Contributor

@adbridge Is that ciarmcom bot buggy? I don't see why it would ask network teams for review.
Platform drivers should go directly to HAL team.

@adbridge
Copy link
Contributor

@SeppoTakalo The review bot is only as good as the data fed to it. The current mappings live here https://github.com/ARMmbed/mbed-os-ci-scripts/blob/master/reviewers/config/reviewers.json. Please feel free to raise a PR to add/correct anything that is missing/wrong or needs updating. Thanks.

@SeppoTakalo
Copy link
Contributor

@adbridge Don't see my team in that line https://github.com/ARMmbed/mbed-os-ci-scripts/blob/master/reviewers/config/reviewers.json#L109

Why would it trigger so many reviewers?

@adbridge
Copy link
Contributor

@SeppoTakalo In this case i suspect it is because of the merge commit added to this PR which will thus have touched many files and the internal Git API is seeing that.

@embeddedteam103 Please do not submit merge commits to PRs. Please instead rebase and repush.

@MarceloSalazar MarceloSalazar requested a review from a team May 16, 2019 13:53
@MarceloSalazar
Copy link

Please wait from @ARMmbed/team-st-mcd confirmation before merging this.

@0xc0170 0xc0170 removed request for a team May 21, 2019 11:21
@0xc0170
Copy link
Contributor

0xc0170 commented May 21, 2019

@embeddedteam103 I believe there is one commit that should not be here (can you please rebase to clean up the history - the fix is just one commit?)

Explains why so many reviewers, I removed them.

@0xc0170
Copy link
Contributor

0xc0170 commented May 21, 2019

@ARMmbed/team-st-mcd Please review

@0xc0170
Copy link
Contributor

0xc0170 commented May 31, 2019

@ARMmbed/team-st-mcd Please review

Any update? I'll run CI meanwhile

@mbed-ci
Copy link

mbed-ci commented May 31, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 self-requested a review June 3, 2019 13:58
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 3, 2019

Resolve #10589 for STM32F4 targets.

Set to needs: work as feedback in 10589 was provided

@jeromecoutant
Copy link
Collaborator

Hi
This PR will be closed if #10804 is approved

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 11, 2019

This PR will be closed if #10804 is approved

Closing, 10804 going ahead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: STM32 HAL SPI Slave MOSI data in shift when CS is NC
8 participants