Skip to content
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

Improve ci #221

Merged
merged 10 commits into from
May 22, 2019
Merged

Improve ci #221

merged 10 commits into from
May 22, 2019

Conversation

Emilgardis
Copy link
Member

@Emilgardis Emilgardis commented May 17, 2018

This pr includes

  • Support for the changes done in v0.13
  • --toolchain foo to specify what toolchain to build the chips with (makes building locally easier, no need to rebuild svd2rust-regress)
  • Add support for --chip XXX YYY
  • Added validators to chip, manufacturer and architecture
  • -vv will now output all stderr logs
  • Added --list to quickly troubleshoot what test cases will be run
  • A patched ATSAMD21G18A svd, thanks to @wez

Open "issues"

  • Should we ignore case when filtering?
  • Should a --chip XXX always run even if it is set to only run on --long-test (and possibly --bad-test)
  • Add a -- command to pass arguments to svd2rust and possibly also a way to pass arguments to cargo check
  • Colorize output
  • Add regress:beta test to gitlab-runner
  • Move test cases to a or multiple config file(s) (e.g json/toml/yaml)

cc @jamesmunns

@Emilgardis Emilgardis requested a review from jamesmunns May 17, 2018 12:32
@jamesmunns
Copy link
Member

Hey @Emilgardis, ACKing this as seen, looks good overall, will have more time tomorrow (2018-05-21) to look at this.

I'd add one open issue: IMO It probably makes more sense to move all the test cases to one or more json/toml/yaml files rather than keeping it in code.

use std::io::prelude::*;
use std::path::PathBuf;
use std::process::{Command, Output};
use tests::TestCase;

static CRATES_ALL: &[&str] = &["bare-metal = \"0.1.0\"", "vcell = \"0.1.0\""];
static CRATES_ALL: &[&str] = &["bare-metal = \"0.2.0\"", "vcell = \"0.1.0\""];
Copy link
Member

Choose a reason for hiding this comment

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

@Emilgardis is this info stored anywhere in svd2rust that we could use to prevent dual maintenance here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The only place where this can be fetched from is the docs. We could also make svd2rust output $PWD/Cargo.toml needed via a flag

@jamesmunns
Copy link
Member

Overall LGTM, if we can dedupe the dependent crates somehow, that would be nice, also feel free to open up one or more follow-up issues for the todos if you would like to go ahead and merge this.

jamesmunns
jamesmunns previously approved these changes May 21, 2018
svd_url: None,
should_pass: false,
run_when: Never,
svd_url: Some("https://raw.githubusercontent.com/wez/atsamd21-rs/master/svd/ATSAMD21G18A.svd"),
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be a specific commit

@mathk
Copy link

mathk commented Feb 21, 2019

bors try

@bors
Copy link
Contributor

bors bot commented Feb 21, 2019

🔒 Permission denied

Existing reviewers: click here to make mathk a reviewer

@mathk
Copy link

mathk commented Feb 21, 2019

bors try

@bors
Copy link
Contributor

bors bot commented Feb 21, 2019

try

Merge conflict

@mathk mathk added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. S-waiting-on-bors Status: Waiting on bors to run and complete tests. labels Feb 21, 2019
@mathk
Copy link

mathk commented Feb 21, 2019

Ping from triage: @Emilgardis can you rebase/merge.

@Disasm Disasm requested a review from a team as a code owner March 16, 2019 16:46
@Disasm
Copy link
Member

Disasm commented Mar 16, 2019

@jamesmunns what is the current status of this PR? Does it need some discussion/changes or can it be merged right now?

@therealprof
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request May 22, 2019
@bors
Copy link
Contributor

bors bot commented May 22, 2019

try

Build succeeded

@Disasm Disasm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels May 22, 2019
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, let's land this and take it from there.

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request May 22, 2019
221: Improve ci r=therealprof a=Emilgardis

This pr includes

- Support for the changes done in v0.13
- `--toolchain foo` to specify what toolchain to build the chips with (makes building locally easier, no need to rebuild `svd2rust-regress`)
- Add support for `--chip XXX YYY`
- Added validators to chip, manufacturer and architecture
- `-vv` will now output all stderr logs
- Added `--list` to quickly troubleshoot what test cases will be run
- A patched ATSAMD21G18A svd, thanks to @wez 

## Open "issues"
- Should we ignore case when filtering?
- Should a `--chip XXX` always run even if it is set to only run on `--long-test` (and possibly `--bad-test`)
- Add a `--` command to pass arguments to `svd2rust` and possibly also a way to pass arguments to `cargo check`
- Colorize output
- Add regress:beta test to gitlab-runner
- Move test cases to a or multiple config file(s) (e.g json/toml/yaml)

cc @jamesmunns 

Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
Co-authored-by: Vadim Kaushan <admin@disasm.info>
@bors
Copy link
Contributor

bors bot commented May 22, 2019

Build succeeded

@bors bors bot merged commit e6cefa5 into rust-embedded:master May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants