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

feat: add strict mode #86

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pimlie
Copy link
Contributor

@pimlie pimlie commented Mar 30, 2019

Ref: #81

Strict mode is currently only used for toggling compromised lock behaviour

currently used for toggling compromised lock behaviour
@codecov
Copy link

codecov bot commented Mar 30, 2019

Codecov Report

Merging #86 into master will increase coverage by 1.70%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #86      +/-   ##
===========================================
+ Coverage   98.29%   100.00%   +1.70%     
===========================================
  Files           4         6       +2     
  Lines         176       205      +29     
  Branches       45        50       +5     
===========================================
+ Hits          173       205      +32     
+ Misses          3         0       -3     
Impacted Files Coverage Δ
lib/lockfile.js 100.00% <100.00%> (+2.45%) ⬆️
test/util/wait.js 100.00% <100.00%> (ø)
lib/mtime-precision.js 100.00% <0.00%> (ø)

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 1a478a4...c19d934. Read the comment docs.

@satazor
Copy link
Contributor

satazor commented Apr 3, 2019

@pimlie I would prefer to not add any option and actually make this the default. This means that the PR would be much simpler by just changing this to not throw:

return releasedCallback &&

@pimlie
Copy link
Contributor Author

pimlie commented Apr 3, 2019

@satazor What about still removing the lock when its compromised? I guess you are opposed to that?

The reason I added it because I actually like the idea that both processes will generate a warning untill we figured out why the lock was compromised in the first place.

@satazor
Copy link
Contributor

satazor commented Apr 3, 2019

If the lock has been compromised, it usually means that someone else got the lock. It would be bad to delete the lock in that situation.

@pimlie
Copy link
Contributor Author

pimlie commented Apr 3, 2019

Updated as requested :)

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