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

add contributors #40

Merged
merged 3 commits into from
Sep 25, 2024
Merged

add contributors #40

merged 3 commits into from
Sep 25, 2024

Conversation

tbo47
Copy link
Owner

@tbo47 tbo47 commented Sep 24, 2024

No description provided.

Copy link
Collaborator

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

You can also add our names to

Copyright (c) 2022-2023 Thibaut Lassalle and dagre-es contributors

Although it would mean every single person that copies code from dagre-es would need to have a massive copyright header!

@tbo47
Copy link
Owner Author

tbo47 commented Sep 24, 2024

You can also add our names to

Copyright (c) 2022-2023 Thibaut Lassalle and dagre-es contributors

Although it would mean every single person that copies code from dagre-es would need to have a massive copyright header!

What about just leaving it like this:

Copyright (c) 2022-2024 dagre-es contributors

@aloisklink
Copy link
Collaborator

What about just leaving it like this:

IANAL (I am not a lawyer), but I think under the MIT license, we might need to ask everybody that contributed to this repo to re-license it without the Thibaut Lassalle in the copyright statement. To be honest, I don't think anybody would mind (other than @tbo47 😆), but it's not worth adding legal ammo, just in case somebody who contributed worked for @oracle or another litigious company.

Adding to it is fine, removing stuff from it is a bit more legally dubious.

@tbo47
Copy link
Owner Author

tbo47 commented Sep 24, 2024

What about just leaving it like this:

IANAL (I am not a lawyer), but I think under the MIT license, we might need to ask everybody that contributed to this repo to re-license it without the Thibaut Lassalle in the copyright statement. To be honest, I don't think anybody would mind (other than @tbo47 😆), but it's not worth adding legal ammo, just in case somebody who contributed worked for @oracle or another litigious company.

Adding to it is fine, removing stuff from it is a bit more legally dubious.

Do you mind adding/removing what you think is right to this PR?

LICENSE.md Outdated Show resolved Hide resolved
Co-authored-by: Alois Klink <[email protected]>
@tbo47
Copy link
Owner Author

tbo47 commented Sep 25, 2024

@aloisklink do you need more time for testing before I release a 7.0.11?

@tbo47 tbo47 merged commit dd5b6fe into main Sep 25, 2024
4 checks passed
@aloisklink
Copy link
Collaborator

@aloisklink do you need more time for testing before I release a 7.0.11?

Yep, sorry.

I'm testing this with mermaid-js/mermaid and between dagre v7.0.10 and v7.0.11, I'm seeing a lot of visual differences:

Git-Graph-diagram-Should-render-subgraphs-with-title-margins-and-edge-labels diff

I haven't yet investigated exactly why there's differences, but I bet we've probably missed something in #34 that isn't causing an error, but is changing some behaviour somewhere.

@tbo47
Copy link
Owner Author

tbo47 commented Sep 25, 2024

Maybe we have to do a lot more of defensive checks.

When I look at this code https://github.com/lodash/lodash/blob/main/src/has.ts
It is testing if the object is not null.

Or maybe it's the forEach that we are doing wrong https://github.com/lodash/lodash/blob/main/src/forEach.ts#L29
_.forEach({ 'a': 1, 'b': 2 } is not the same as { 'a': 1, 'b': 2 }.forEach

@tbo47
Copy link
Owner Author

tbo47 commented Sep 27, 2024

I think I'm going to revert all the lodash related changes. I don't see the point of removing them and take the risk of having regressions. @aloisklink

@tbo47 tbo47 mentioned this pull request Sep 28, 2024
This was referenced Sep 30, 2024
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