Skip to content

add clang-format option from env variable#382

Merged
IvanGrigorik merged 2 commits intomainfrom
grigorik/format-option
Feb 28, 2026
Merged

add clang-format option from env variable#382
IvanGrigorik merged 2 commits intomainfrom
grigorik/format-option

Conversation

@IvanGrigorik
Copy link
Copy Markdown
Collaborator

As it was said in this issue:
#380 (comment)

We should have total flexibility over our tool, including formatting of intermediate files, if needed
This PR adds PK_FORMAT env variable dependency, so when export PK_FORMAT=1 we will do clang-format pass on our IR files

@IvanGrigorik IvanGrigorik force-pushed the grigorik/format-option branch from f8bac1d to ceddfbb Compare February 27, 2026 23:01
Copy link
Copy Markdown
Collaborator

@kennykos kennykos left a comment

Choose a reason for hiding this comment

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

Can we add documentation for the environment variable before this is merged in?

@IvanGrigorik
Copy link
Copy Markdown
Collaborator Author

Can we add documentation for the environment variable before this is merged in?

added

Copy link
Copy Markdown
Collaborator

@kennykos kennykos left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me.

@IvanGrigorik IvanGrigorik merged commit 00c8a5f into main Feb 28, 2026
17 checks passed
@IvanGrigorik IvanGrigorik deleted the grigorik/format-option branch February 28, 2026 01:47
self.lib_path_env: str = "PK_KOKKOS_LIB_PATH"

self.format: bool = False
self.format: bool = os.getenv("PK_FORMAT") is not None
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.

Just out of curiosity: this also formats with PK_FORMAT=0?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great catch, sorry I missed this in my code review. It should be something like os.getenv("PK_FORMAT", False) , which will default to False if not available

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.

3 participants