Skip to content

svd-parser 0.11 #542

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

Merged
merged 3 commits into from
Oct 6, 2021
Merged

svd-parser 0.11 #542

merged 3 commits into from
Oct 6, 2021

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Oct 5, 2021

No description provided.

@burrbull burrbull requested a review from a team as a code owner October 5, 2021 04:39
@rust-highfive
Copy link

r? @Emilgardis

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Oct 5, 2021
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

Could we try using --const-generics and --strict on everything as option in the feature matrix? Would increase CI but I think it's worth it

@burrbull
Copy link
Member Author

burrbull commented Oct 5, 2021

Could we try using --const-generics and --strict on everything as option in the feature matrix? Would increase CI but I think it's worth it

Not all SVDs feel good with --strict. But we can maximize usage of these flags.
Could you fix issue with multiple flags? I'm not good in bash.

@Emilgardis
Copy link
Member

Emilgardis commented Oct 5, 2021

Ill get to it later today CET

@Emilgardis
Copy link
Member

Maybe just removing the bash script and completely use svd2rust-regress is a good thing to do as well. Not sure if that would be in the scope of this pr though.

therealprof
therealprof previously approved these changes Oct 5, 2021
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.

FWIW, looks good to me.

@Emilgardis
Copy link
Member

https://github.com/rust-embedded/svd2rust/blob/69a869c06a5b171028b5cf04fd3a9d7e63b2c9d5/.github/bors.toml

needs to be changed, I'm not sure if we have a policy on how to gather multiple results and gate bors on it, but I'd prefer to have a workflow that needs: ci-linux

@Emilgardis

This comment has been minimized.

bors bot added a commit that referenced this pull request Oct 5, 2021
@Emilgardis
Copy link
Member

for clarity, the try run shouldn't fail because of esp-rs/esp32#47 recently being merged

@Emilgardis
Copy link
Member

ugh, changelog check is not ran on trying 🤷
bors try-

also merges CI to one status check
@Emilgardis
Copy link
Member

bors try

testing the combined ci check

bors bot added a commit that referenced this pull request Oct 5, 2021
@bors
Copy link
Contributor

bors bot commented Oct 5, 2021

try

Build succeeded:

Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

lgtm

leaving bors r=me for @burrbull, not sure if this is considered done, but looks to be to me

@burrbull
Copy link
Member Author

burrbull commented Oct 6, 2021

bors r=Emilgardis

@bors
Copy link
Contributor

bors bot commented Oct 6, 2021

Build succeeded:

@bors bors bot merged commit cc9fd06 into master Oct 6, 2021
@bors bors bot deleted the svg_ng2 branch October 6, 2021 04:09
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. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants