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

replace archy with a specific implementation, refactor timetree #211

Merged
merged 8 commits into from
Jun 24, 2023

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jun 23, 2023

archy is 9 years old and you can not find the repo on github anymore.
this PR replaces the need for archy and removes some double processing

added also a unit test as #205 got also fixed, i presume.

Edit:
removed unreachable code and added jsdoc types.
test coverage 100% for time-tree.js

Checklist

@Uzlopak Uzlopak requested a review from Eomm June 23, 2023 18:29
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina
Copy link
Member

it might be worth moving that archy clone in another repo, I think we use it in find-my-way as well

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jun 23, 2023

@mcollina
I checked it and find-my-way does not use archy. Also this is an adaptation for the specific need of avvio.

@Eomm

Can you have a look regarding bug #205?

@Eomm
Copy link
Member

Eomm commented Jun 24, 2023

it might be worth moving that archy clone in another repo, I think we use it in find-my-way as well

I have a fork - https://github.com/Eomm/node-archy
I may publish it with a bit rework

@Uzlopak Uzlopak changed the title replace archy with a specific implementation replace archy with a specific implementation, refactor timetree Jun 24, 2023
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jun 24, 2023

I removed unreachable code, added jsdoc typings

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Uzlopak Uzlopak merged commit 4e4a22d into master Jun 24, 2023
@Uzlopak Uzlopak deleted the prettyPrint branch June 24, 2023 21:36
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