Skip to content

Conversation

@moschlar
Copy link
Contributor

No description provided.

@moschlar moschlar force-pushed the formatting-options branch from 39b898e to 2311423 Compare August 15, 2025 13:14
@cf-bottom
Copy link

Thanks for submitting a PR! Maybe @craigcomstock can review this?

Copy link

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Good to keep existing behavior with defaults. 👍

Copy link
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

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

Hello!

Thank you for opening a pull request. In general we don't want too much customizability in this formatter, since the goal of it is to make consistent looking policy files. (If everyone is customizing their formatters then they're no longer consistent, or perhaps less consistent than if they all used the same style).

With that said, I think it's okay to allow for customizable line lengths, at least for now.

In order to get this merged, please address the following:

  • Remove the commit concerning customizing indent
  • Change the command line option to --line-length
  • Change the places where you are sending args objects to functions, use an explicit parameter with an appropriate name instead (line_length).
  • Add a simple shell test where you run the tool with different --line-length values, and check that it works correctly.

Let me know if you have any questions!

@moschlar moschlar force-pushed the formatting-options branch from 2311423 to cc7b876 Compare August 26, 2025 12:39
@moschlar moschlar changed the title Add some customizable formatting options for cf policy files Add --line-length parameter to format subcommand for cf policy files Aug 26, 2025
@moschlar moschlar requested a review from olehermanse August 26, 2025 12:41
@moschlar moschlar force-pushed the formatting-options branch from cc7b876 to e74c980 Compare August 27, 2025 12:02
Copy link
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

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

This looks ready now, thanks for your contribution! (In the future, feel free to open an issue to discuss something before implementing :) )

@olehermanse olehermanse merged commit 059fd6f into cfengine:main Aug 28, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants