Skip to content

Ci wippersnapper boards local txt #217

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

Draft
wants to merge 14 commits into
base: ci-wippersnapper
Choose a base branch
from

Conversation

tyeth
Copy link
Member

@tyeth tyeth commented Jul 24, 2025

Adds support for --boards-local-txt argument, with file specified or defaulting to current working directory.
Copies specified file, if found, to the board support package for the platform.

Flag also enables support for boards.txt.local file in example/sketch folders, and/or board build target specific .<platform>.boards.local.txt in example folders

@tyeth
Copy link
Member Author

tyeth commented Jul 25, 2025

Probably need to delete any example specific boards.local.txt and revert to the root one after each example runs / is tested.

Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

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

Tyeth and I went over this over slack, all changes OK 🏁

"""
try:
local_app_data_dir = os.environ.get('HOME', '')
data_dir = None
Copy link
Member

Choose a reason for hiding this comment

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

place local_app_data_dir and data_dir above the try, it's referenced lower in the code too. If it fails, you dont have a reference to these when you need data_dir again on L479

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe this makes sense (with regards to line 479 of original review), as the try covers the entire function so the variables won't be reused in the event of a caught error, can you explain it to me?

@tyeth tyeth requested a review from brentru August 18, 2025 18:27
Comment on lines +47 to +57
if sys.argv[sys.argv.index("--boards-local-txt") + 1].startswith("--"):
if os.path.exists("boards.local.txt"):
boards_local_txt = "boards.local.txt"
else:
sys.stderr.write("Error: --boards-local-txt option requires a path to boards.local.txt file (or to be present in current directory)\n")
sys.exit(1)
# get the boards.local.txt file from the command line (index exists checked earlier)
if not os.path.exists(sys.argv[sys.argv.index("--boards-local-txt") + 1]):
sys.stderr.write("Error: boards.local.txt file does not exist\n")
sys.exit(1)
boards_local_txt = sys.argv[sys.argv.index("--boards-local-txt") + 1]
Copy link
Member Author

@tyeth tyeth Aug 18, 2025

Choose a reason for hiding this comment

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

second if should be elif but negated with the line57 body, and first if should confirm there is a period (".") in the argument that follows --boards-local-txt, and finally the error as else body.

@tyeth tyeth removed the request for review from brentru August 19, 2025 13:14
@tyeth tyeth marked this pull request as draft August 19, 2025 13:14
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