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

Some miscellaneous ci and documentation additions #256

Conversation

nialov
Copy link
Collaborator

@nialov nialov commented Dec 18, 2023

Hey, went through the package for the purposes of general build file and documentation cleaning. Currently a draft until I have tested the new CI additions. None of the changes are required and some are subjective changes so if something should be kept as is (like keeping requirements.txt), just let me know and I will revert the change here.

  • build: clean python dependency management
  • docs: cleaned and updated documentation
  • ci: add docker build job
  • ci(tests): use use poetry
  • build: enable calling toolkit as module

@nmaarnio
Copy link
Collaborator

Hey, I think these look great improvements at a glance. Shoot when with a review request when you're ready!

@nialov
Copy link
Collaborator Author

nialov commented Dec 19, 2023

I will do some minor changes still, like making a new Dockerfile instead of messing up the old so if someone is using it they can continue. Also sorry about making formatting changes to the documentation, they are a bit annoying to review but changes to contents itself are minor. There mostly were some old instructions on how to develop etc. that I changed to reflect the current guidance. I will also make a comment here with a list of the actual changes I made.

Reverted the changes to the original Dockerfile so if
someone is using it, it still works as expected.
@nialov
Copy link
Collaborator Author

nialov commented Dec 19, 2023

Changes:

  • Made tests.yaml use poetry for installing dependencies instead of requirements.txt.
  • Removed requirements.txt and setup.py. As I understood, these are not used outside tests.yaml which I changed. This will remove the need to separately maintain requirements.txt.
  • Cleaned and updated documentation
  • Added docker.yaml GitHub Action that uses the new Dockerfile-docs for testing poetry installation with pytest and builds the documentation using mkdocs. It also uploads the built pdf as an artifact so it can be downloaded. This checks that the docker and documentation is built successfully. pdf artifact from this pull request e.g. https://github.com/GispoCoding/eis_toolkit/actions/runs/7250994119/artifacts/1121907364
  • Added eis_toolkit/__main__.py so that you can do python3 -m eis_toolkit to run the toolkit from command-line.
  • Updated docs/dependency_licenses.md

These should be the most major changes. Ready for review. As said before, some changes are subjective and anything can be reverted. Just wanted to show the changes that I would personally make.

@nialov nialov marked this pull request as ready for review December 19, 2023 12:46
@nialov nialov requested a review from nmaarnio December 19, 2023 12:47
Copy link
Collaborator

@nmaarnio nmaarnio left a comment

Choose a reason for hiding this comment

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

I think all of the modifications/additions are good, and can't find any issues. So, ready to merge for my part! But since you updated GH actions, I wonder if we should enable the macos-latest test as part of this PR and see if it works now? I disabled it previously because it suddenly started failing everytime, and didn't really look into the matter

@nialov
Copy link
Collaborator Author

nialov commented Dec 20, 2023

macos tests ran successfully.

@nmaarnio nmaarnio merged commit 04428df into GispoCoding:master Dec 20, 2023
6 checks passed
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.

2 participants