Skip to content
This repository has been archived by the owner on Oct 9, 2024. It is now read-only.

ci: use prettier on docs md files #217

Merged
merged 2 commits into from
Feb 10, 2022
Merged

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Feb 5, 2022

Follow-Up: #215

Increases the scope of markdown files prettier will clean-up to include the docs directory and includes changes to those files made by prettier.

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

This seems to continue the tradition of not giving a darn about review requests like #177 but I'll still try. So I want to block this until #181 is reverted. Changing these linting settings was never discussed or accepted by the team.

Copy link

@cidrblock cidrblock left a comment

Choose a reason for hiding this comment

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

Approving this based on the general philosophy that formatters free up our time and CPU cycles so they can be used on user-facing features and resolving user-facing issues.

That said, I will defer to @tomaciazek @ganeshrn and @priyamsahoo here to decide if this gets merged.

@tomaciazek
Copy link
Contributor

tomaciazek commented Feb 7, 2022

This seems to continue the tradition of not giving a darn about review requests like #177 but I'll still try. So I want to block this until #181 is reverted. Changing these linting settings was never discussed or accepted by the team.

@ssbarnea, I agree with @webknjaz that you should have waited for his review before merging #181, as he did explicitly state opposition to these changes in #177.

So I want to block this until #181 is reverted.

@webknjaz, are you saying that you'll approve this one once #181 is reverted? But that will inevitably break/change markdown files on every save, as explained in #177 ;)

@ssbarnea, @webknjaz could you please create a discussion similar to ansible/team-devtools#28, where everyone can express their opinion in a well-structured format, since this is obviously a polarizing topic? The changes introduced here aren't urgent, so I don't see a reason not to take time to discuss them.

Follow-Up: ansible#215

Increases the scope of markdown files prettier will clean-up to
include the docs directory and includes changes to those files made
by prettier.
@webknjaz
Copy link
Member

webknjaz commented Feb 9, 2022

are you saying that you'll approve this one once #181 is reverted?

I'm just saying that merging stuff with rejections or no reviews is a bad tone unless agreed offline. These things cascade and become harder to git revert, especially as new changes are made on top of that. The proper process would be re-submitting that and discussing the PR, including getting all the explanations necessary recorded.
I was merely calling this out because it's not up to the bar we want to hold ourselves to. FWIW it's been discussed at a team meeting so nothing left to do right now.

For now, I've lost interest in trying to review this as it doesn't feel like it'd matter anymore. This is why I'm just going to approve it.

@ssbarnea ssbarnea merged commit ab3c95e into ansible:main Feb 10, 2022
@ssbarnea ssbarnea deleted the prettier/md branch February 10, 2022 08:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants