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

New cookiecutter #29

Merged
merged 14 commits into from
Feb 26, 2024
Merged

Conversation

CodeWithEmad
Copy link
Member

@CodeWithEmad CodeWithEmad commented Dec 26, 2023

This includes several improvements to the overall functionality of the cookiecutter:

  • Added Cookiecutter hooks that validate input data.
  • Expanded support for various licenses.
  • Implemented a new documentation format.
  • Added GitHub Actions for new plugins. Close Include basic CI setup for new plugins #7
  • Added a dev mode to extra entries.
  • Consolidated all configurations in one place for improved readability.

This is a result of a couple of sleepless nights. I wanted to explore all the features of the Cookiecutter package. Feel free to experiment and add more elements to it.

Close #7 #30

@CodeWithEmad
Copy link
Member Author

I wanted to remind you that this PR has been open for over a month. Before merging it, I would appreciate a thorough review from anyone.
@kdmccormick
@regisb

@kdmccormick
Copy link
Contributor

Sorry for the radio silence @CodeWithEmad . Reviewing this is in my backlog.

@CodeWithEmad
Copy link
Member Author

No worries. Thanks in advance.

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

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

I expected this would be a big review... which is why I unconsciously postponed it as much as I could 😓 sorry about that!

Despite my many comments, I agree with the general direction, and I think it's going to be a great improvement. Thanks a lot Emad!

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Show resolved Hide resolved
cookiecutter.json Outdated Show resolved Hide resolved
hooks/pre_gen_project.py Outdated Show resolved Hide resolved
hooks/pre_gen_project.py Outdated Show resolved Hide resolved
{{ cookiecutter.package_name }}/setup.py Outdated Show resolved Hide resolved
{{ cookiecutter.package_name }}/setup.py Show resolved Hide resolved
@CodeWithEmad
Copy link
Member Author

Thanks for the review, @regisb, and sorry for the late response. I was on a vacation. Will look into the comments as soon as I get the chance.

@CodeWithEmad
Copy link
Member Author

CodeWithEmad commented Feb 12, 2024

That took more than I expected.
Kindly have a look, @regisb.
Also, about pkg_resources issue on Python 3.12, what's the best approach? people can still select older tutor versions and I think we didn't backport those changes for older tutor versions.

@regisb
Copy link
Contributor

regisb commented Feb 13, 2024

This is how pkg_resources should be handled:
overhangio/tutor#977
overhangio/tutor-minio@3873ae0

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I'd like to have someone else's opinion :) @kdmccormick?

We currently support 4 types of licenses: AGPLv3, Apache 2.0, BSDv3, and MIT. if the user selects "Not open source", then this file will be removed in the post_generate hook.
- overhang domain changed to edly.
- cookiecutter document changed to main page
- using code-blocks for better readability
- followed the openedx doc conventions for rst files
with these we can sanitize our data before and modify files after cookiecutter was generated.
as of tutor >=17.0.0, there's no need to run `tutor config save` after enable/disable a plugin.
@CodeWithEmad
Copy link
Member Author

CodeWithEmad commented Feb 19, 2024

I've included the pkg_resources deprecation fix in this MR
@kdmccormick

Copy link
Contributor

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

CHANGELOG.md Outdated
Comment on lines 13 to 20
- [Improvement] Happy New Year!
Fix compatibility issue with Python 3.12 by removing dependency on `pkg_resources`.
Cookiecutter hooks to check input data validation.
Various licenses support.
New documentation format.
GitHub Actions for new plugins.
`dev` mode added to extra entries.
(by @CodeWithEmad).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [Improvement] Happy New Year!
Fix compatibility issue with Python 3.12 by removing dependency on `pkg_resources`.
Cookiecutter hooks to check input data validation.
Various licenses support.
New documentation format.
GitHub Actions for new plugins.
`dev` mode added to extra entries.
(by @CodeWithEmad).
- [Improvement] Happy New Year!
- Fix compatibility issue with Python 3.12 by removing dependency on `pkg_resources`.
- Cookiecutter hooks to check input data validation.
- Various licenses support.
- New documentation format.
- GitHub Actions for new plugins.
- `dev` mode added to extra entries.
(by @CodeWithEmad).

Without the bullet points, this will all render on one line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sharp eyes 👀
fixed.

- ignore all dirs starts with tutor-contrib-
- __pycache__ added
The long_description_content_type parameter is added to the setup() function to explicitly specify that the long description content is in reStructuredText format.
pkg_resources is a package that is unavailable in python 3.12, unless
setuptools is explicitely installed. Turns out, there are replacement
functions coming from importlib_resources, which can be obtained from
the importlib-resources pypi package. This package will be installed
with tutor starting from 17.0.2.
@CodeWithEmad
Copy link
Member Author

Should I consider closing #7 after this got merged?

@kdmccormick
Copy link
Contributor

Indeed we should consider that closed. Thanks Emad!

@CodeWithEmad
Copy link
Member Author

CodeWithEmad commented Feb 21, 2024

Awesome, so I merge this if there's nothing else to add. @DawoudSheraz

@DawoudSheraz
Copy link

Awesome, so I merge this if there's nothing else to add. @DawoudSheraz

Hi, I will add my review today. Sorry for the delay on my end.

@CodeWithEmad
Copy link
Member Author

@DawoudSheraz just a friendly reminder. I don't wanna keep this PR open any longer.

@CodeWithEmad CodeWithEmad merged commit 5d5241d into overhangio:master Feb 26, 2024
@regisb
Copy link
Contributor

regisb commented Feb 27, 2024

Thanks again for your work Emad :)

@CodeWithEmad
Copy link
Member Author

Anytime, @regisb.

@CodeWithEmad CodeWithEmad deleted the new-cookiecutter branch February 27, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Include basic CI setup for new plugins
4 participants