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

Board names changed to be valid python identifiers #6979

Closed
wants to merge 4 commits into from

Conversation

bill88t
Copy link

@bill88t bill88t commented Oct 1, 2022

This PR is a followup to #6856.

Basically I replaced all "-" and "." with "_".
As for the 8086_commander, I opted to just swap the words around.

Additional requirements for this change to go well:

  • The same changes will have to be done on the -org repo.
    I will do that pr if you decide to go ahead with this.
  • This and the -org PRs will have to be merged EXACTLY when 8.0.0 releases.
    Because the board names will remain unchanged on 7.x.
  • When new boards are being ported, a check (perhaps github action)
    should be in place to prevent boards from having invalid python identifiers as their name.

Fixes #7267.

@bill88t
Copy link
Author

bill88t commented Oct 1, 2022

I am very confused as to why the usb id check failed.. I did mv it correctly, checked my history..

@bill88t bill88t marked this pull request as ready for review October 1, 2022 13:15
@jepler
Copy link
Member

jepler commented Oct 1, 2022

Looks like the PR needs to changed tools/ci_check_duplicate_usb_vid_pid.py as well.

PS this script might be one place to add enforcement of the policy for board names to only be valid Python identifiers.

@jepler
Copy link
Member

jepler commented Oct 1, 2022

also it looks like at least one "-" is left after this, boards/espressif_kaluga_1-3/board.c

@bill88t
Copy link
Author

bill88t commented Oct 1, 2022

I will try to create a tools/ci_check_board_identifiers.py, that checks all the board names.

@jepler
Copy link
Member

jepler commented Oct 1, 2022

I was thinking the ci_check_duplicate script could just have it added, but a separate step/script is fine too.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 1, 2022

There is an board alias mechanism which could be used to ease the transition, though I'm not sure if it would produce multiple picture entries in circuitpython.org.

It's also true that the old names are used in the directory tree structure in S3: https://adafruit-circuit-python.s3.amazonaws.com/index.html?prefix=bin. If we rename the directories there, we need to track down any links besides the ones in circuitpython-org. There may be many in the Learn Guides.

If we are going to do a big renaming, we might also consider renaming the older adafruit boards that don't have adafruit_ as a prefix.

I a bit worried whether there's too much churn created by this. One can always take the given board name and substitute _ for - using str.replace() at run-time. I agree it would be good to enforce a standard naming going forward.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 4, 2022

I am going to make this a draft while we cogitate on this.

@dhalbert dhalbert marked this pull request as draft October 4, 2022 01:43
@tannewt
Copy link
Member

tannewt commented Nov 15, 2022

I think a CI check for new boards is best. Is there tooling that needs the python identifier property for all boards?

@dhalbert
Copy link
Collaborator

We discussed closing draft issues that are stuck in the CircuitPython meeting on 2022-11-28 in In the Weeds.

Closing for now. Can be reopened. Tracking in issue #7267, which will remain open.

@dhalbert dhalbert closed this Nov 28, 2022
@bill88t bill88t deleted the board-valid-identifiers branch March 4, 2023 16:27
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.

Change board names to be valid Python identifiers
4 participants