Skip to content

DO NOT MERGE: tests: add some questionable but valid platform names - #117

Open
lengau wants to merge 1 commit into
mainfrom
work/valid-platform-names
Open

DO NOT MERGE: tests: add some questionable but valid platform names#117
lengau wants to merge 1 commit into
mainfrom
work/valid-platform-names

Conversation

@lengau

@lengau lengau commented Mar 31, 2025

Copy link
Copy Markdown
Contributor
  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run make lint && make test?

This is for discussion about what, if any, limits we want to provide for platform names.

@lengau
lengau force-pushed the work/valid-platform-names branch from 1c5e938 to f02ffee Compare March 31, 2025 21:44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some thoughts:

  1. Platform names are used for the container name. Craft-providers sanitizes platform names before creating a container, so this isn't a concern.
  2. Platform names are used in the resulting artifact. So it may be prudent to only allow characters allowed in file paths. We could add a few extra characters and replace them. I think we are already doing this with @ or : symbols in Charmcraft.
  3. For usability, we could set a maximum limit. The artifact filename should be less than 255 characters (a common max), so this could drive the maximum limit.
  4. We could set a reasonable ruleset. Unless there is a real need to support the platform names in this PR, I'd be fine for something along the lines of:

Can contain alphanumerics, :, @, - and _, must start and end with an alphanumeric, max length is 64 characters.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I want 🇿🇦 to be a valid platform name.

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