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

Change default layers to use lowercase directories #4505

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

seldridge
Copy link
Member

Change directories for default layers to use lowercase output directories. While this is independently likely a better choice, it is a somewhat pragmatic, as these directory names match the legacy names of firtool's extract-test-code feature (which was a custom SiFive pass for the SFC before being open-sourced in firtool). This will help us land an extract-test-code replacement without having to disturb build flows. Secondarily, and independently, lower case directories are likely better.

There are two alternative changes which were rejected:

  1. The layers could be renamed to be lowercase. Generally, it's better to have objects and classes be uppercase.

  2. The automatic directory names could be mangled to be lowercase as opposed to the exact layer names.

Release Notes

Change directories for default layers to be lowercase.

Change directories for default layers to use lowercase output directories.
While this is independently likely a better choice, it is a somewhat
pragmatic, as these directory names match the legacy names of `firtool`'s
extract-test-code feature (which was a custom SiFive pass for the SFC
before being open-sourced in `firtool`).  This will help us land an
extract-test-code replacement without having to disturb build flows.
Secondarily, and independently, lower case directories are likely better.

There are two alternative changes which were rejected:

1. The layers could be renamed to be lowercase.  Generally, it's better to
   have objects and classes be uppercase.

2. The automatic directory names could be mangled to be lowercase as
   opposed to the exact layer names.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge requested review from jackkoenig and rwy7 November 13, 2024 22:44
@seldridge seldridge added API Modification Backend Code Generation Affects backend code generation, will be included in release notes and removed API Modification labels Nov 13, 2024
Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

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

LGTM. I think this is better default behavior

@seldridge seldridge enabled auto-merge (squash) November 13, 2024 23:37
@seldridge seldridge merged commit 0bb1bce into main Nov 13, 2024
14 checks passed
@seldridge seldridge deleted the dev/seldridge/change-default-layer-directories branch November 13, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Code Generation Affects backend code generation, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants