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 class and toString() for dependencies #8

Closed
wants to merge 1 commit into from
Closed

Add class and toString() for dependencies #8

wants to merge 1 commit into from

Conversation

jeroen
Copy link
Member

@jeroen jeroen commented Nov 24, 2019

This is a small change that gives dependency instances type Dependency such that we can add a toString() method to the prototype, which restores the original string.

This mostly restores backward compatibility with rdesc-parser 2.x :)

function Dependency(package, version) {
this.package = package;
if(version)
this.version = version;
Copy link
Contributor

Choose a reason for hiding this comment

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

So we don't want to separate the >=? FWIW in theory it can also be == or <= although that's hardly ever used.

Copy link
Member Author

@jeroen jeroen Nov 24, 2019

Choose a reason for hiding this comment

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

The parser leaves the >= part in the version. So it already contains e.g. "> 3.0"

Copy link
Contributor

Choose a reason for hiding this comment

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

I am asking if we should separate those or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok. I don't think so, I don't see how the >= by itself is meaingful without the number.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, if you want to check if a version requirement holds, then you need to separate them every time, so we might already in the parser. Basically inside the parens it is a tiny DSL, so we might as well parse it.

}

Dependency.prototype.toString = function() {
return this.package + (this.version ? " (" + this.version + ")" : "");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we explicitly compare to undefined? Maybe, IDK.

@jeroen
Copy link
Member Author

jeroen commented Nov 25, 2019

Maybe the complexity if introducing a type doesn't weigh up to the added complexity right now. I'm closing this for now, maybe revisit later.

@jeroen jeroen closed this Nov 25, 2019
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.

2 participants