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

Experimental ci with github actions #2269

Merged
merged 15 commits into from
Jun 3, 2020

Conversation

weiznich
Copy link
Member

If that actually works I'm proposing moving all of our ci/automation
to this service, because of:

  • Integration with github seems to be vastly better
    • No need to click n times to go to logs
    • Rerun a failed job is easier
    • Maybe? Inline error messages in PR's
  • Error messages are automatically extracted
  • I think they have 20 free concurrent jobs, azure has only 10 (so
    faster CI)

(Most of the config is copied from wundergraph)

@weiznich weiznich force-pushed the try/github_actions branch 15 times, most recently from d267175 to e75936d Compare January 15, 2020 22:24
@JohnTitor
Copy link
Member

fail-fast: false might be useful here.

@weiznich weiznich force-pushed the try/github_actions branch 7 times, most recently from 9240d7d to 1e11e76 Compare January 16, 2020 20:28
if: success()
uses: JamesIves/github-pages-deploy-action@releases/v3
with:
ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }}
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to create this token somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

with:
ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }}
BRANCH: gh-pages # The branch the action should deploy to.
FOLDER: target/doc # The folder the action should deploy.
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 means we need to change the documentation link on our web page

@weiznich
Copy link
Member Author

This seems to work now modulo the expected nightly failures, so it's ready to get comments.

@weiznich weiznich requested a review from a team January 16, 2020 20:56
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Overall looks good since most config is ported from pipelines' and actions seem to work fine (the log is hard a bit to check though). About doc.yml, I'd like to hear sgrif's opinion.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@weiznich
Copy link
Member Author

Just got a mail that says we need to change our azure pipeline config till March 23. (Have to update some runner image versions till then at least.)
I would say we should try to get this PR done till then, so we don't have to handle that other change.

weiznich and others added 6 commits May 26, 2020 17:47
If that actually works I'm proposing moving all of our ci/automation
to this service, because of:

* Integration with github seems to be vastly better
    + No need to click n times to go to logs
    + Rerun a failed job is easier
    + Maybe? Inline error messages in PR's
* Error messages are automatically extracted
* I think they have 20 free concurrent jobs, azure has only 10 (so
faster CI)

(Most of the config is copied from wundergraph)
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@weiznich
Copy link
Member Author

weiznich commented May 27, 2020

Related things that should be fixed before merging:

  • Badges section in Cargo.toml (Likely also for other crates)
  • Badges section in Readme.md
  • Add a subpage behind the documentation link on the web page to list correctly the documentation for different versions
  • Backport this pr at least to the 1.4.x branch (for documentation and testing there)

@JohnTitor
Copy link
Member

Badges section in Cargo.toml (Likely also for other crates)

crates.io doesn't display CI-related badge now so we could just remove them.

* Lower all dependencies to `-Z minimal-versions` in CI
* Raise the following public dependencies:
    * bigdecimal to 0.0.13 to drop support for old num versions
    * chrono to 0.4.1 because it pulls in a really old num from before
      the rust 1.0 release
    * pq-sys to 0.4.0 because 0.3.x pulls in a old bindgen version
    * mysqlclient-sys to 0.3.0 for the same reason
* Bump this internal dependencies:
    * syn to 1.0.1 because we require at least that version
    * num-* to 0.2.0 to match bigdecimal version
    * bcrypt to 0.2.0 because it would pull in rustc-serialize
      otherwise
    * tempfile to 3.0.0 because otherwise it pulls in winapi 0.0.1!!!
    * bitflags to 1.2.0 because of deprecation warnings regarding to try!
@weiznich
Copy link
Member Author

@diesel-rs/reviewers Should be ready for a final review

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Looks good! Left some comments, feel free to apply them if you like.

.github/workflows/doc.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@weiznich weiznich merged commit fb8dddd into diesel-rs:master Jun 3, 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.

3 participants