Code commit and review process

[pbrier]

Would it be a suggestion to implement a "formal" code commit and review process?
e.g. something like this:
https://github.com/energia/Energia/wiki/Bug-fix-and-new-feature-review-process

We could use topic branches, or at least have some staging in the process from feature/bugfix to master branch (e.g. using a "development" branch)

The github wiki pages could be used to document the process (along with coding styles and other considerations)

[bogdanm]

Hi Peter,

On Sun, Oct 20, 2013 at 1:14 PM, Peter Brier notifications@github.comwrote:

Would it be a suggestion to implement a "formal" code commit and review
process?
e.g. something like this:

https://github.com/energia/Energia/wiki/Bug-fix-and-new-feature-review-process

We could use topic branches, or at least have some staging in the process
from feature/bugfix to master branch (e.g. using a "development" branch)

The github wiki pages could be used to document the process (along with
coding styles and other considerations)

One possible issue with this approach is that we accept pull requests from
two different sources: github.com and mbed.org. Defining a common policy
would probably be difficult due to the differences in the interfaces and
usability of these two sites, while defining two different policies might
be confusing. Is there something in particular that bothers you with our
current process?

Thanks,
Bogdan

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/87
.

[pbrier]

Currently using the HEAD master branch sometimes breaks a build, due to fixes that are pulled in, that are
not sufficiently scrutinized (e.g. see Issue #94 and issue #95, due to pulling in Issue #91).

I would propose that before anything is put in to the master branch, at least:

• The code is build for all compilers, targets and configurations (it should not fail)
• All samples are build (and auto tests performed)
• The code is reviewed by someone else than the author

Does this make sense?

[emilmont]

Hi Peter,
by default we do check the build only with ARMCC and the process is launched manually.

Does this make sense?

Of course, it makes sense.
We are planning to trigger this process automatically, triggered by github pull requests and to add all the compilers. This would have detected the issue introduced for the GCC compiler (#94).

Cheers,
Emilio

[aethaniel]

Hi Emilio,

Sorry to disturb but when this validation process would be available?

Does it also mean that any pull request, for example bringing new devices port, would have to take ARMCC into consideration?
Obviously a patch on code previously validated on ARMCC would have to be working.

My concern is really about providing new devices (and boards) with ARM GCC based projects.

Cheers

[emilmont]

Hi Aethaniel,

when this validation process would be available?

We should have it before the end of the year.

Does it also mean that any pull request, for example bringing new
devices port, would have to take ARMCC into consideration?

No, it will be configurable, similarly to what we are doing for the build releases:
https://github.com/mbedmicro/mbed/blob/master/workspace_tools/build_release.py#L29

It is totally fine to submit a "pull request" adding support for a new target even if it is initially tested only with GCC.

Cheers,
Emilio

[matthewelse]

Hi Everyone (this is probably more useful for Emilio),

Have you considered using something like Travis-CI, which could build all of the code when a push is made, and then provide you with a nice graphic that you could include in the README... The only problem might be that it probably doesn't have all of the embedded compilers. If Travis-CI doesn't include the necessary compilers, you could possibly look at something like Buildbot (used by OpenCV), which might allow more flexibility.

Thanks,

Matthew

[mbednotifications]

It'd be very easy to setup Travis test with GCC and could even run on Qemu
too. I'm more the happy to help! I suppose GCC would be rather enough to
catch most errors, while there might be some compiler-specific cases, we
could still get a green or red light for trivial pull-requests much
quicker. If merging and testing that is done by the maintainers is
currently decoupled, then we could probably make master 80% more stable by
adding Travis... By the way, one can deploy Travis on their site too
(perhaps this could be done to provide ARMCC).

[matthewelse]

Just out of interest, who sent that? It just says mbed notifications...

[matthewelse]

I've just got a travis proof of concept working - see 3da2549. The build succeeded on all of the targets that officially support GCC_ARM, however they use Ubuntu, so there's not much we can do about ARM_CC at the moment.

[matthewelse]

Just done a test pull request as well, just to make sure that Travis is doing its job properly, and so far it seems to be working well... 👍

[aethaniel]

Indeed, Travis-CI seems really interesting.
Why do you have this error: E: Unable to locate package gcc-arm-none-eabi ?
Is it because we can't install external packages?

[matthewelse]

I only had that error on either the first commit, which I had to fix, and when I purposely broke it on a branch to make sure it was working ok

[matthewelse]

The most recent commit on the master branch passed. See this: https://travis-ci.org/matthewelse/mbed/builds/13145173

[aethaniel]

I saw. Great job, Matthew. Thanks for sharing this.

[matthewelse]

No problem... It could be a bit interesting getting ARM_CC to run though. Maybe wine, or something like that would run it?