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

New options: no file, archive and git deps #169

Merged
merged 4 commits into from
Dec 5, 2019

Conversation

miripiruni
Copy link
Contributor

Motivation

I want to be able check dependencies that unreliable.

Developers can use deps from git branches, tags or even commit sha. Such deps can be corrupted (after force push, repo deletion or move from user to organisation).

It‘s more reliable to use npm packages (since it’s hard to unpublish).

So, I introduced options:
— no-file-dependencies (from local directory)
— no-archive-dependencies (from tar or zip)
— no-git-dependencies (including commits sha, tags and branches)

The same options for devDeps.

Checklist

  • Unit tests have been added
  • Specific notes for documentation, if applicable

@@ -744,6 +744,120 @@ describe('dependency-audit Unit Tests', () => {
});
});

describe('doVersContainGitRepository method', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question about dependency-audit.test.js: why we need this tests since it’s duplicate tests from test/unit/rules/*?

Now I carefully write code and tests at the same style as already written. But for me it’s a little bit overtested :)

Copy link
Owner

Choose a reason for hiding this comment

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

Hey @miripiruni - great question. You are right that it is probably a bit overkill. I figure it is better to be safe 🤓 . I'll think about adjustments for v5.

@tclindner
Copy link
Owner

Thank you for opening a PR! I wanted to let you know that I won’t be able to look at this for the next week. I’m looking forward to reviewing.

@miripiruni
Copy link
Contributor Author

@tclindner thank you for quick response. I’ll wait.

@miripiruni
Copy link
Contributor Author

polite ping

@tclindner
Copy link
Owner

Hey @miripiruni I apologize for the delay! I was traveling ✈️ 😄

This is fantastic! 🎉 These rules are awesome. You did a fantastic job on them. 🙌 I'll get a release cut shortly.

Thank you again! I really appreciate all of your effort.

@tclindner tclindner merged commit 1b90077 into tclindner:master Dec 5, 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