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 CI for volunteers page #42

Merged
merged 1 commit into from
Jan 14, 2022
Merged

Add CI for volunteers page #42

merged 1 commit into from
Jan 14, 2022

Conversation

Logout22
Copy link
Contributor

Leveraging free (as in beer) resources for open-source CI, we can start improving the software quality of the volunteers portal and thereby make changes easier and bugs rarer going forward.

If you all agree that codecov is a good idea, I will need to add the upstream project there before merging.

@johanvdw
Copy link
Member

Thanks a lot - sorry for only seeing this now (as we run fosdem only once a year, I don't check the repo very often.). I'll review.

@Logout22
Copy link
Contributor Author

Sure, no problem!

ci/Dockerfile Outdated
@@ -0,0 +1,8 @@
FROM python:3.7-buster
Copy link
Member

Choose a reason for hiding this comment

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

I just bumped the server to bullseye, we probably should base our ci on that as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -1,4 +1,6 @@
arabic-reshaper==2.1.1
astroid==2.5.6
Copy link
Member

Choose a reason for hiding this comment

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

perhaps it would be better to create a requirements-dev file which includes dependencies for development, and not for deploy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I split the two apart, which also removed the merge conflict. That should suffice for now. In the long run, poetry may be an option, but one step at a time. :)


jobs:
test_job:
runs-on: ubuntu-latest
Copy link
Member

Choose a reason for hiding this comment

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

It should not make a big difference, but we tend to use debian rather than ubuntu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get "Can't find any online and idle self-hosted runner in the current repository, account/organization that matches the required labels: 'debian-latest'" when I try that.

It seems like GitHub has decided to stick with Ubuntu for all of their Linux CI hosts: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners

So, unfortunately, we will be limited to the Docker container for Debian.

Copy link

Choose a reason for hiding this comment

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

SourceHut offers Debian VM images and I'd be glad to integrate it with this repo if it is desired.

Copy link
Member

@johanvdw johanvdw Jan 14, 2022

Choose a reason for hiding this comment

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

The os doesn't really matter as we are using a virtualenv anyway. But thanks for the suggestion anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed @johanvdw , but @McSinyx if you have a plan how to do that integration, I would be fine with it.

Copy link

Choose a reason for hiding this comment

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

Doing it would require either trusting me for owner privilege or creating a paying account for FOSDEM ($20/year; builds.sr.ht got hit by cryptocurrency miners and decided to require payment earlier than initially planned). Personally I'm OK with either but it's up for the maintainers to decide if it's worth it.

set -o errexit
set -o nounset

sed "\
Copy link
Member

Choose a reason for hiding this comment

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

Rather than using sed, I would provide a specific settings_sqlite.py and a settings_postgres.py . I'm okay doing this now, but in any case, I think we can improve how this is done:
#44

This change includes the first automated test for the system.
@Logout22
Copy link
Contributor Author

Logout22 commented Dec 4, 2021

I squashed all my changes to make merging and future review easier.

@johanvdw johanvdw merged commit 77b75b7 into FOSDEM:master Jan 14, 2022
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