Skip to content

Conversation

@marcoieni
Copy link
Contributor

Closes #132

Questions:

  • why is cross needed?
  • Do you think I covered everything Travis was doing?

@marcoieni marcoieni requested a review from a team as a code owner December 26, 2020 11:22
@marcoieni
Copy link
Contributor Author

marcoieni commented Dec 26, 2020

You can see a preview of the action running on my branch: https://github.com/MarcoIeni/svd/actions

@burrbull
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Dec 26, 2020
@brainstorm
Copy link
Contributor

brainstorm commented Dec 26, 2020

why is cross needed?

One thing that cross offers is (some) QEMU MCU emulation... I guess that using cross was a way to future-proof CI so that very simple emulation tests could be implemented in the future?

Other than that, LGTM 👍

@marcoieni
Copy link
Contributor Author

I think we have to change this:

"continuous-integration/travis-ci/push",

@marcoieni
Copy link
Contributor Author

marcoieni commented Dec 26, 2020

I guess that using cross was a way to future-proof CI so that very simple emulation tests could be implemented in the future?

If we are not using it at the moment we can remove the use-cross flag in ci.yml. We can add it in the future if needed.

@burrbull
Copy link
Member

cc @therealprof

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the conventions outlined in https://github.com/rust-embedded/wg/blob/master/ops/post-transfer.md

Specifically the trigger rules and bors.toml are not sufficient yet.

@marcoieni
Copy link
Contributor Author

marcoieni commented Dec 26, 2020

I think we have to change this:

"continuous-integration/travis-ci/push",

I completely removed the status field of bors, because maybe it is not required with github actions. Is this right?
Can you check if this field is present in some other project that uses bors and github actions?

EDIT: I saw therealprof comment, I will do it today!

@burrbull
Copy link
Member

@therealprof can you check bors settings/permissions for this repo?

@bors
Copy link
Contributor

bors bot commented Dec 26, 2020

try

Timed out.

burrbull
burrbull previously approved these changes Dec 26, 2020
@therealprof
Copy link
Contributor

@therealprof can you check bors settings/permissions for this repo?

Can do but I think it should work already just fine.

@marcoieni
Copy link
Contributor Author

Should I remove "S-waiting-on-team" from bors.toml? Because it is not present in the template.

@marcoieni marcoieni marked this pull request as ready for review December 26, 2020 19:21
@therealprof
Copy link
Contributor

Should I remove "S-waiting-on-team" from bors.toml? Because it is not present in the template.

Don't bother. Doesn't do any harm.

@therealprof
Copy link
Contributor

Mind squashing your commits?

@marcoieni
Copy link
Contributor Author

marcoieni commented Dec 26, 2020

Done!
For the future, have you considered adding use_squash_merge = true boolean variable to bors.toml? See https://bors.tech/documentation/

@therealprof
Copy link
Contributor

Having separate commits can be very useful so auto-squashing is not a good idea.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 26, 2020

Build succeeded:

@bors bors bot merged commit 5915c96 into rust-embedded:master Dec 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Is Continuous Integration working?

5 participants