Skip to content

Use cargo-xtask in CI#207

Merged
jonas-schievink merged 2 commits intonrf-rs:masterfrom
jonas-schievink:xtask
Sep 4, 2020
Merged

Use cargo-xtask in CI#207
jonas-schievink merged 2 commits intonrf-rs:masterfrom
jonas-schievink:xtask

Conversation

@jonas-schievink
Copy link
Copy Markdown
Contributor

Tests can be run with cargo test -p xtask, no bash required. cargo test does not yet work, I'll see if it's possible to get that working after this PR.

My plan is to use cargo-xtask as a general automation tool for releases (allowing automatically bumping versions, html_root_url, and the changelog).

@jonas-schievink
Copy link
Copy Markdown
Contributor Author

Oh, and this removes the non-working i2s-peripheral-demo since we now check that every example has test metadata (cc @kalkyl)

@kalkyl
Copy link
Copy Markdown
Contributor

kalkyl commented Sep 2, 2020

Oh, and this removes the non-working i2s-peripheral-demo since we now check that every example has test metadata (cc @kalkyl)

Nice! Yeah i'm just finishing up a new PR tonight where I changed the implementation to work along the lines of the embedded-dma traits instead, so there's an updated working version of this demo in that PR :)

@jonas-schievink
Copy link
Copy Markdown
Contributor Author

Oh, awesome!

Comment thread .cargo/config
]

[build]
target = "thumbv7em-none-eabihf"
Copy link
Copy Markdown
Contributor

@Yatekii Yatekii Sep 2, 2020

Choose a reason for hiding this comment

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

Why is this gone? I would assume this is still relevant for development?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sort of. We now explicitly pass the right target in CI, and I've found this default to be a bit confusing since it doesn't work on nRF51, the nRF52810, and the nRF9160.

Demos can add their own .cargo/config if they want though, that should make development easier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, I guess it does work on the nRF9160, but it's not usually the target you want to use there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm ok, so I guess you suggest just using the build script during development I guess :) I always hit cargo build out of habit =D but the change & its reasoning is fair enough for me!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you can still do that if you cd to the demo project you're editing and add a .cargo/config there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't care about the demo projects ;) I care about building the HAL if I change something (which hasn't happened in a log time).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The HALs should all build even for x86_64 (at least that's how docs.rs generates documentation for them)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does cortex-m really build for x86 targets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it goes out of its way to ensure that

@jonas-schievink
Copy link
Copy Markdown
Contributor Author

Going to merge this tomorrow unless there are objections :)

@jonas-schievink jonas-schievink merged commit 142279c into nrf-rs:master Sep 4, 2020
@jonas-schievink jonas-schievink deleted the xtask branch September 4, 2020 13:19
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.

3 participants