adafruit / cookiecutter-adafruit-circuitpython

Cookiecutter template for Adafruit's CircuitPython libraries.
MIT License
22 stars 37 forks source link

RFC: Allow Options For Different Target Bundles #84

Closed sommersoft closed 3 years ago

sommersoft commented 4 years ago

Addresses #73.

Adds the following choices:

I recorded some demos to show it in action: https://youtu.be/Ukli5XFNAvE

Submitting as a draft for RFC purposes, and I also need to adjust the tests when its finalized.

Tagging @Gadgetoid for input.

kattni commented 4 years ago

@sommersoft I'm entirely on board for this. Let me know if you need any assistance.

siddacious commented 4 years ago

@sommersoft looks good to me!

Gadgetoid commented 4 years ago

Definitely a step in the right direction! I'll have to find something new to cookiecut.

askpatrickw commented 3 years ago

@sommersoft and @kattni This addresses some of the items I mention in #100, any chance this is going to make it into main?

kattni commented 3 years ago

@askpatrickw As discussed on Discord, I really appreciate that you're going to step up to get this finished! Please ping me in this thread with any questions you have. Thanks!

askpatrickw commented 3 years ago

@kattni as we talked about, I removed the Travis pieces. I think as-is this is a good stepping stone to address all the other elements I mention in #100 or to do other community v. Adafruit specific config.

Testing: I created a repo from this and looked around, it looked good to my eye. A frequent user might notice things I did not.

kattni commented 3 years ago

@askpatrickw Can you send me a zip of a generated repo please? I'll take a look at it.

askpatrickw commented 3 years ago

@kattni here you go

AdafruitLibraryCookie.zip CommunityLibraryCookie.zip

In each zip there is a .txt file showing the options I chose when running

kattni commented 3 years ago

@askpatrickw setup.py is still Adafruit-centric. Two options here, one: make it less so for Community version. Two, and I think this one may make more sense: not include it in the community version. I don't think the Community bundle is deploying to PyPI automatically anyway, and if folks want to deploy to PyPI, they need to put in credentials and all of that. So.... is that possible? I don't know enough about how this works to answer that question.

kattni commented 3 years ago

@FoamyGuy This is the PR to keep an eye on for updating the cookiecutter section in the Sharing a Library guide. Thanks again for taking this on!

kattni commented 3 years ago

I replied in #100 as well.

askpatrickw commented 3 years ago

@kattni & @FoamyGuy It is not possible to do a logic tree (if A then ask this question) flow of prompts. I think 99% of what we both felt could be improved can be handled as-is so I'm not going to go down that path.

This isn't done, but these are my notes from the work I did tonight. I will figure out the merge issue tomorrow.

ToDo's and Thoughts:

Fixes so far

cookiecutter.json

Project Folder

Library.py

README.rst

Setup.py

code_of_conduct.md

sample.py

Docs - index.rst

API.rst

askpatrickw commented 3 years ago

OK... all the big things are done. Three known things I need to deal with:

Fixes:

cookiecutter.json

README.rst

requirements.txt

setup.py

library/docs/conf.py

sommersoft commented 3 years ago

@askpatrickw I resolved the merge conflicts. Just a little out-of-sync-ness with other merges. Just need to pull from sommersoft/bundle_aware on your end and you should be gtg moving forward. Let me know if you need any help with adjusting the tests.

askpatrickw commented 3 years ago

Thank you @sommersoft 🙏

askpatrickw commented 3 years ago

We're getting closer, I'm going to ping the team tomorrow during the day on 1 and 2. I'm stumped.

Remaining Work

  1. What is .rst.license? (I just don't know) Should they all add the library author?
  2. The github_user is not required if you select Community bundle. That is bad. It breaks a lot of things downstream. I'm looking into that. The workaround is not have a default value, but it makes more sense for Adafruit bundle libraries to have the defaults defined to ensure consistency.

Fixed

sommersoft commented 3 years ago

@askpatrickw for number 2, a pre-generation hook to validate github_user could allow for the default value to remain but still ensure a valid entry is entered.

https://cookiecutter.readthedocs.io/en/1.7.2/advanced/hooks.html#example-validating-template-variables

FoamyGuy commented 3 years ago

I tested this out tonight and I believe I do have a good grasp on the differences in prompts and files generated to change the learn guide when we are ready.

askpatrickw commented 3 years ago

Thanks @sommersoft that does atleast block the cookiecutter form generating bad files. It is not as good as the behavior I am wishing for which is for cookiecutter to re-prompt like it does for Author. @kattni you should chime in on this.

What follows is my command line session. Notice that first I put nothing for author and then it makes me enter something. But for github_user, I can put no entry. We check if it is in ["", None] and if it is, then we Error out in a way that prevents the cookiecutter from generating invalid template files.

Select target_bundle:
1 - Adafruit
2 - Community
Choose from 1, 2 [1]: 2
library_name: NOUSER
library_description []:
library_keywords []:
author:                        <--- I didn't supply a value, it prompted me again
author: safafsg           <--- I supply a value, it continues
library_prefix []:
github_user []:           <--- I didn't supply a value
company []:
requires_bus_device []:
requires_register []:
other_requirements []:
pypi_release [no]:
Select default_branch:
1 - master
2 - main
Choose from 1, 2 [1]:
ERROR: github_user is a required field.
ERROR: Stopping generation because pre_gen_project hook script didn't exit successfully
Hook script failed (exit status: 1)

We have two choices:

  1. This solution above where sometimes for community bundle templates they have to re-enter everything again. But, for Adafruit libraries you get the happy enter, enter, use the defaults.
  2. Change the github_user prompt so it has no default for either type of library. The risk here is someone making an adafruit library and putting in their own github user name which be easy enough to fix, but also make invalid files.

I think the first is the best worst choice because it prevents someone getting a day or two into something and realizing they need to fix their library repo.

askpatrickw commented 3 years ago

OK... All done but the review.

Last round of Changes:

askpatrickw commented 3 years ago

@kattni and @FoamyGuy I think I'm done. Thanks to @sommersoft for your help along the way too!!

FoamyGuy commented 3 years ago

Also possibly remove this section if the user chose no docs https://github.com/adafruit/cookiecutter-adafruit-circuitpython/blob/e48e92131509314d8ebc4d59153dadd575357a04/%7B%25%20if%20cookiecutter.library_prefix%20%25%7D%7B%7B%20cookiecutter.library_prefix%20%7C%20capitalize%20%7D%7D_%7B%25%20endif%20%25%7DCircuitPython_%7B%7B%20cookiecutter.library_name%7D%7D/.github/workflows/build.yml#L64

askpatrickw commented 3 years ago

@askpatrickw can we change the way it finds files to run pylint on here:

https://github.com/adafruit/cookiecutter-adafruit-circuitpython/blob/e48e92131509314d8ebc4d59153dadd575357a04/%7B%25%20if%20cookiecutter.library_prefix%20%25%7D%7B%7B%20cookiecutter.library_prefix%20%7C%20capitalize%20%7D%7D_%7B%25%20endif%20%25%7DCircuitPython_%7B%7B%20cookiecutter.library_name%7D%7D/.github/workflows/build.yml#L55

for community bundle projects? Maybe use the value from library_prefix if we can?

That's a good catch! I think *{{ cookiecutter.library_name | lower }}.py would always work (unless they change the name or make it module (in a folder)..

askpatrickw commented 3 years ago

Also possibly remove this section if the user chose no docs

https://github.com/adafruit/cookiecutter-adafruit-circuitpython/blob/e48e92131509314d8ebc4d59153dadd575357a04/%7B%25%20if%20cookiecutter.library_prefix%20%25%7D%7B%7B%20cookiecutter.library_prefix%20%7C%20capitalize%20%7D%7D_%7B%25%20endif%20%25%7DCircuitPython_%7B%7B%20cookiecutter.library_name%7D%7D/.github/workflows/build.yml#L64

Another good catch! 100% that should be conditional.

askpatrickw commented 3 years ago

OK @FoamyGuy I fixed the two things you found: Added a check for docs folder before building docs. Made pylint scan all .py files except conf.py and setup.py so contributors don't have to deal with those files.

Also fixed a long-line in the readme.rst.

Thanks again for your testing and code review!