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

Add Github Actions for CI #390

Merged
merged 2 commits into from
Dec 23, 2020
Merged

Add Github Actions for CI #390

merged 2 commits into from
Dec 23, 2020

Conversation

adamgreig
Copy link
Contributor

@adamgreig adamgreig commented Dec 22, 2020

This is a first attempt at resolving #389. I expect it will need some fine-tuning; feedback welcome.

I believe I've included everything from the current Travis config except for checking the bench, which seems to be broken at the moment anyway.

Edit: It seems like building on 1.28.0 is broken as it can't parse the Cargo.toml file of cfg-if or other dependencies, although the README suggests that should be the MSRV. It looks like even once that issue is resolved, the use of MaybeUninit requires 1.36, which does build OK, so I've set a check at 1.36 in the CI and updated README.

Edit 2: It looks like the test without std is failing due to a whole bunch of errors; I've removed it for now.

Edit 3: Tests just won't pass without std or alloc, so I've swapped it to only check the build for that feature-set.

@jordens
Copy link

jordens commented Dec 22, 2020

Nice!
Feel free to bump the msrv. 1.28 was because of artiq only. No need to block on that.

@adamgreig adamgreig marked this pull request as ready for review December 22, 2020 12:46
@adamgreig adamgreig marked this pull request as draft December 22, 2020 12:52
@adamgreig
Copy link
Contributor Author

Ok, this is ready for review. At the moment Travis is still enabled too, but we could remove it in this PR.

@whitequark
Copy link
Contributor

Thank you for tackling this! As far as I'm concerned this is ready. You can just get rid of Travis.

@whitequark
Copy link
Contributor

whitequark commented Dec 22, 2020

I'm not sure who can actually merge this PR though; for some reason I don't have the permission bit for manually merging to protected branches, and the Travis required status will never pass now.

@adamgreig
Copy link
Contributor Author

I guess @jordens? I was previously an admin of the organisation but apparently no longer, so I can't disable the Travis required check either.

@whitequark
Copy link
Contributor

I'm pretty sure I had repo admin at one point but seemingly no longer? No idea what's up with that.

@therealprof
Copy link
Contributor

I also don't have the necessary permissions.

@jordens
Copy link

jordens commented Dec 23, 2020

I don't know why or who set these permissions up, I don't think it makes sense and it doesn't seem desirable to me. I have made the three of you owners.

@therealprof
Copy link
Contributor

I don't know why or who set these permissions up, I didn't I don't think it makes sense and it doesn't seem desirable to me. I have made the three of you owners.

Thanks!

@therealprof therealprof merged commit 86333b0 into master Dec 23, 2020
@therealprof therealprof deleted the gha branch December 23, 2020 08:56
@whitequark
Copy link
Contributor

To ensure the long-term health of the project, I went ahead and did two things:

  1. I went through the GitHub organization audit log, determined the cause of the permission change that prevented this PR from being merged, and made the minimal amount of changes to ensure that this sort of disruption will not happen again.
  2. As you may have noticed, I've only had a limited amount of time to spend on smoltcp lately. As the original author of the codebase, I believe that @Dirbaio would make a great smoltcp maintainer, and he is also in a good position to be one. I have invited him to the organization, and he has my blessing in determining the future direction of the project to the same degree as I do.

I think smoltcp has a great future ahead of it, and I look forward to seeing it evolve in 2021 with a new maintainer.

@Dirbaio Dirbaio mentioned this pull request Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants