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

refactor: change exposed methods and accepted types #12

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

jobo322
Copy link
Member

@jobo322 jobo322 commented Feb 8, 2024

BREAKING CHANGE:

  • treeSimilarity now only accepts Tree objects
  • createTree expects an object {x: [],y: []}

fix!: treeSimilarity only expects tree
chore: treeSimilarity could accepts null
    because it is the way the method stops the     recursive execution
fix!: createTree expects a object {x,y}
feat: add type structures
chore: adapt README
@jobo322 jobo322 linked an issue Feb 8, 2024 that may be closed by this pull request
10 tasks
@targos
Copy link
Member

targos commented Feb 8, 2024

The message for the changelog is wrong (breaking change is not the rename of the method). You should remove the ! from the title and add a BREAKING CHANGE: section with the actual breaking changes.

@jobo322
Copy link
Member Author

jobo322 commented Feb 8, 2024

@targos I have a question about the use of .d.ts to store the types. Is it needed to point to this file in the package.json or is it just enough with the importation in the jsdoc?

@targos
Copy link
Member

targos commented Feb 8, 2024

You need to create a "types" key in the package.json that points to the file, and also include the file in the distribution (files array in package.json).

@jobo322 jobo322 changed the title fix!: rename getSimilarity to treeSimilarity fix: treeSimilarity n createTree refactored and types added Feb 8, 2024
@jobo322
Copy link
Member Author

jobo322 commented Feb 8, 2024

The message for the changelog is wrong (breaking change is not the rename of the method). You should remove the ! from the title and add a BREAKING CHANGE: section with the actual breaking changes.

It is correct now?

@targos targos changed the title fix: treeSimilarity n createTree refactored and types added refactor: change exposed methods and accepted types Feb 8, 2024
@targos targos merged commit 3ec97d8 into main Feb 8, 2024
7 checks passed
@targos targos deleted the 11-breaking-changes-to-do branch February 8, 2024 16:05
@targos
Copy link
Member

targos commented Feb 8, 2024

No, please have a look at the message I reworded: 3ec97d8

  • It is not a fix
  • We only talk about the breaking changes from the perspective of the end user

@targos
Copy link
Member

targos commented Feb 8, 2024

I'm sorry, the format was actually wrong. Here is a new version that works: ec9318f

We cannot have an empty line for BREAKING CHANGE:
In general, I think the best way to solve this is to make more than one commit, so each commit has its description and BREAKING CHANGE: section.

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.

Breaking changes to do
2 participants