Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove default indent #24

Merged
merged 1 commit into from
May 4, 2018
Merged

Remove default indent #24

merged 1 commit into from
May 4, 2018

Conversation

mohd-akram
Copy link
Contributor

No description provided.

@jonschlinkert jonschlinkert merged commit 786ebf1 into jonschlinkert:master May 4, 2018
@Lewiscowles1986
Copy link

Lewiscowles1986 commented Jul 22, 2023

Surely this is a breaking change?

This was released with a patch version, but the change is to the output of this function; so if someone uses this library, will it not affect their tests?

Feels like it should have been at-least 1.3.x not a continuation of 1.2.x
As it's end user facing, it might even be enough for a major version.

@doowb
Copy link
Collaborator

doowb commented Jul 22, 2023

@Lewiscowles1986 thanks for catching this. This PR was merged into master over 5 years ago and I didn't notice that it wasn't published to NPM.

I'll get a new patch released that reverts this PR.

@Lewiscowles1986
Copy link

😬 what about just releasing a major, or I contribute some docs about the breakage for a 1.2.5?

My comment wasn't meant to do more than ignite discussion; I was not trying to tell what you should do; but I did get an update notification for this on a repo, and I read the changelog, and was curious.

I was kinda hoping you'd tell me I read it wrong 😂

@doowb
Copy link
Collaborator

doowb commented Jul 24, 2023

I was kinda hoping you'd tell me I read it wrong

@Lewiscowles1986 you read it correctly and you're right that it's a breaking change. Since 1.2.4 is intentionally a patch to fix #32 , the default indent change should be in a new version. I get annoyed by patches having breaking changes, so I'm glad you caught that.

I see you created a PR to re-add this change. Thanks for doing that and I'll try to get it merged and published soon.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants