Skip to content
This repository has been archived by the owner on Feb 5, 2018. It is now read-only.

fix(lib): getting .git from parent dir #74

Merged

Conversation

standy
Copy link
Contributor

@standy standy commented Apr 4, 2017

Closes #72

@codecov-io
Copy link

codecov-io commented Apr 4, 2017

Codecov Report

Merging #74 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #74   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines         123    126    +3     
=====================================
+ Hits          123    126    +3
Impacted Files Coverage Δ
lib/getGitFolder.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a579a3d...f683599. Read the comment docs.

Copy link
Collaborator

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

LGTM

"avatar_url": "https://avatars3.githubusercontent.com/u/750319?v=3",
"profile": "https://github.com/standy",
"contributions": [
"bug",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use code instead of bug

Copy link
Collaborator

@spirosikmd spirosikmd left a comment

Choose a reason for hiding this comment

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

Thank you for the fix! I see you added a new package, but yarn.lock hasn't been committed? Can you update it? Or am I missing something?

@standy
Copy link
Contributor Author

standy commented Apr 4, 2017

find-parent-dir was already in yarn.lock, since husky from devDependencies uses it.
https://github.com/kentcdodds/validate-commit-msg/blob/master/yarn.lock#L1097

I wonder why library has lockfile, isnt it ignored by package users? Looks like just yet another file to maintain

Copy link
Collaborator

@spirosikmd spirosikmd left a comment

Choose a reason for hiding this comment

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

Ah cool! Didn't notice that! But it is not ignored for development? Every contributor should have the same packages, right?

@standy
Copy link
Contributor Author

standy commented Apr 4, 2017

Having same packages for every contributor does not guarantee that package will work for users, I think that having some diversity would be better for contributors
I know it might be some risks from having different versions inside dependencies, but since lockfile has same risks but for for users, we should rely on semver (and avoid package who do not)

@spirosikmd spirosikmd merged commit e3a81f8 into conventional-changelog-archived-repos:master Apr 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants