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

Ported Buildkite checks to Github actions #210

Merged
merged 33 commits into from
Apr 13, 2021

Conversation

H-Shay
Copy link
Contributor

@H-Shay H-Shay commented Apr 4, 2021

I ported the first of the CI/CL checks pipeline to Github Actions. I just want to make sure this port was correct before I ported the rest of the checks. Let me know if it is and I will update my pull request with the rest of the checks completed. Thank you!
Signed-off-by: Hillery Shay [email protected]

@clokep
Copy link
Member

clokep commented Apr 5, 2021

@H-Shay It looks like this also includes the commits from #194, you probably want to merge the master branch into this branch and then push again!

@H-Shay
Copy link
Contributor Author

H-Shay commented Apr 5, 2021

Alright I pulled down the upstream repository and merged with my fork and pushed again. Hopefully that's better!

@clokep
Copy link
Member

clokep commented Apr 5, 2021

Alright I pulled down the upstream repository and merged with my fork and pushed again. Hopefully that's better!

Looks good now! Thanks!

@clokep clokep requested a review from a team April 5, 2021 16:45
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Thanks for your work; this is a great start! I've left a few comments below.

.github/workflows/pipeline.yml Outdated Show resolved Hide resolved
changelog.d/210.misc Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
changelog.d/210.misc Outdated Show resolved Hide resolved
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

I'm assuming you wanted another review of this PR since you answered all my questions, but apologies if you were still working on this round! And if you were waiting for review, note that hitting the request review button is the best way to communicate that 🙂

That said, this is starting to really shape up! I'll popped some hopefully help notes below.

.github/workflows/changelog_check.yml Outdated Show resolved Hide resolved
.github/workflows/pipeline.yml Outdated Show resolved Hide resolved
.github/workflows/pipeline.yml Outdated Show resolved Hide resolved
.github/workflows/pipeline.yml Outdated Show resolved Hide resolved
Copy link
Member

@anoadragon453 anoadragon453 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 to me other than a few small things!

.github/workflows/pipeline.yml Outdated Show resolved Hide resolved
.github/workflows/changelog_check.yml Outdated Show resolved Hide resolved
.github/workflows/pipeline.yml Outdated Show resolved Hide resolved
.github/workflows/changelog_check.yml Outdated Show resolved Hide resolved
@anoadragon453 anoadragon453 changed the title Ported towncrier checks to Github actions Ported Buildkite checks to Github actions Apr 13, 2021
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Heads up that if you've resolved an issue as part of a pull request comment thread, resolving the thread once discussion has finished and the suggestion has been implemented (or rejected) helps shrink the total vertical space of the PR.

Anyways, this PR looks ready to go 🎉 Thank you for your work on it! 🙂

@anoadragon453
Copy link
Member

Oh, actually, as a very pedantic last step, can I get you to sign off on your changes just in a comment stating your ownership of this code change? Details for doing so are here: https://github.com/matrix-org/sygnal/blob/master/CONTRIBUTING.md#sign-off

@H-Shay
Copy link
Contributor Author

H-Shay commented Apr 13, 2021

Cool! I updated the original PR comment to include a sign-off statement. Thanks for all your helpful feedback!

@anoadragon453 anoadragon453 merged commit 62bdfeb into matrix-org:master Apr 13, 2021
awesome-michael added a commit to Awesome-Technologies/sygnal that referenced this pull request May 6, 2021
Sygnal 0.9.3 (2021-04-22)
=========================

Features
--------

- Prevent the push key from being rejected for temporary errors and oversized payloads, add TTL logging, and support `events_only` push data flag. ([\matrix-org#212](matrix-org#212))
- WebPush: add support for Urgency and Topic header ([\matrix-org#213](matrix-org#213))

Bugfixes
--------

- Fix a long-standing bug where invalid JSON would be accepted over the HTTP interfaces. ([\matrix-org#216](matrix-org#216))
- Limit the size of requests received from HTTP clients. ([\matrix-org#220](matrix-org#220))

Updates to the Docker image
---------------------------

- Remove manually added GeoTrust Root CA certificate from docker image as Apple is no longer using it. ([\matrix-org#208](matrix-org#208))

Improved Documentation
----------------------

- Make `CONTIBUTING.md` more explicit about how to get tests passing. ([\matrix-org#188](matrix-org#188))
- Update `CONTRIBUTING.md` to specify how to run code style and type checks with Tox, and add formatting to code block samples. ([\matrix-org#193](matrix-org#193))
- Document how to work around pip installation timeout errors. Contributed by Omar Mohamed. ([\matrix-org#215](matrix-org#215))

Internal Changes
----------------

- Update Tox to run in the installed version of Python (instead of specifying Python 3.7) and to consider specific paths and folders while running checks, instead of the whole repository (potentially including unwanted files and folders, e.g. the virtual environment). ([\matrix-org#193](matrix-org#193))
- Make development dependencies available as extras. Contributed by Hillery Shay. ([\matrix-org#194](matrix-org#194))
- Update `setup.py` to specify that a minimum version of Python greater or equal to 3.7 is required. Contributed by Tawanda Moyo. ([\matrix-org#207](matrix-org#207))
- Port CI checks to Github Actions. ([\matrix-org#210](matrix-org#210), [\matrix-org#219](matrix-org#219))
- Upgrade development dependencies. Contributed by Omar Mohamed ([\matrix-org#214](matrix-org#214))
- Set up `coverage.py` to run in tox environment, and add html reports ([\matrix-org#217](matrix-org#217))

Change-Id: I14ae821ff2a1562e91fd87ce25f73baaa0b9430b
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.

4 participants