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

Trails #71

Open
wants to merge 29 commits into
base: dev
Choose a base branch
from
Open

Trails #71

wants to merge 29 commits into from

Conversation

GabrielaPC
Copy link
Collaborator

No description provided.

@GabrielaPC GabrielaPC changed the base branch from master to dev June 29, 2022 18:40
package.json Outdated
@@ -52,7 +52,8 @@
"ninja-util": "^1.3.1",
"request": "^2.88.0",
"request-promise-native": "^1.0.8",
"shortid": "^2.2.14"
"shortid": "^2.2.14",
"linked-list": "git+ssh://[email protected]:keg-core/linked-list.git"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this might be a problem. As this is a public library it should not refer no any private dependency.

Copy link
Collaborator

@eltonfss eltonfss left a comment

Choose a reason for hiding this comment

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

I believe the linked-list dependency should be moved to a public repository before being referenced in this package. One alternative would be to include it within this repository.

@marcelomachado marcelomachado requested a review from eltonfss July 1, 2022 01:59
src/datasource/hkdatasource.js Outdated Show resolved Hide resolved
src/trail.js Show resolved Hide resolved
src/trail.js Outdated Show resolved Hide resolved
src/datasource/hkdatasource.js Outdated Show resolved Hide resolved
Copy link
Member

@marcelomachado marcelomachado left a comment

Choose a reason for hiding this comment

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

Why are we putting an entire project inside hklib instead of using the npm package?

If it is really necessary I would prefer to put this in a util folder instead of the src root.

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.

5 participants