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 PR template #6348

Merged
merged 3 commits into from Mar 21, 2018

Conversation

Projects
None yet
5 participants
@0xc0170
Member

0xc0170 commented Mar 13, 2018

Description

Fixing bad markdown choice - PR type is not a task list but just a list of checkboxes

Thanks @bcostm for the report

Pull request type

  • Fix
  • Refactor
  • New target
  • Feature
  • Breaking change

@0xc0170 0xc0170 requested review from cmonr and adbridge Mar 13, 2018

@adbridge adbridge added needs: CI and removed needs: review labels Mar 14, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 15, 2018

Sometimes, kicking off a morph build feels really silly.

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 15, 2018

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 15, 2018

Status wasn't reported.
/morph export-build

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 16, 2018

/morph mbed2-build

@bcostm

This comment has been minimized.

Contributor

bcostm commented Mar 16, 2018

@0xc0170 are you sure this new lines will display correctly the result, I mean without the progress bar appearing on the PR main page ? Because this morning I tried the same syntax in my PR 6379 (i.e. * [ ] Fix...) and the progress bar was still present.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 16, 2018

@bcostm That PR was missing the * before the [ ] and [x]. I just modified that PR's description, and it now looks like this PR.

@bcostm

This comment has been minimized.

Contributor

bcostm commented Mar 16, 2018

I think we misunderstand each other 😄 With your modification I see this 1 of 5 progress bar:

image

And this is what I wanted to remove... For me those tasks list (and progress bar) should be kept for good reasons like waiting reviewers approval, tests, etc...

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 16, 2018

@bcostm Ah, gotcha. For some reason, I recalled seeing that task bar in the PR and thought I noticed it go away with this syntax...

Anyways, @0xc0170 thoughts? A quick look at the GitHub markdown indicates that using any kind of checkboxes will lead to that progress bar.

https://help.github.com/articles/about-task-lists/

@cmonr cmonr added needs: review and removed ready for merge labels Mar 16, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 20, 2018

Thanks @bcostm for the review, have to revise this 🙄

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 20, 2018

Looking at the checkbox possibility, the only way it's task list that we do not want. Therefore I would revert this to the basic list

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

Does not look "fancy" but still keeps the same informative context that is important to us.

@0xc0170 0xc0170 dismissed stale reviews from adbridge and cmonr via e628a06 Mar 20, 2018

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_pr_template branch from 8436095 to e628a06 Mar 20, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 20, 2018

Rebased, updated

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 20, 2018

@bcostm Thoughts?

@bcostm

This comment has been minimized.

Contributor

bcostm commented Mar 21, 2018

Yes it looks fine. Thanks.

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_pr_template branch from e628a06 to 65bb06a Mar 21, 2018

@0xc0170 0xc0170 force-pushed the 0xc0170:fix_pr_template branch from 65bb06a to e2d0eaf Mar 21, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 21, 2018

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@adbridge

I can live with that now as it is :)

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Mar 21, 2018

/morph build

(I aborted the previous build, the code was changed after)

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 21, 2018

Build : SUCCESS

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

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr removed the needs: review label Mar 21, 2018

@cmonr cmonr merged commit 816890d into ARMmbed:master Mar 21, 2018

11 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-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
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

0xc0170 added a commit to 0xc0170/Handbook that referenced this pull request Mar 23, 2018

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