Skip to content

reverting back to working client#20

Merged
vsoch merged 1 commit intomasterfrom
revert/back-to-working-client
Apr 5, 2020
Merged

reverting back to working client#20
vsoch merged 1 commit intomasterfrom
revert/back-to-working-client

Conversation

@vsoch
Copy link
Copy Markdown
Collaborator

@vsoch vsoch commented Apr 5, 2020

@SuperKogito I am so upset I couldn't wait to fix this. We need to talk about the proper review and testing that needs to happen before you push breaking changes like this.

Signed-off-by: vsoch vsochat@stanford.edu

Signed-off-by: vsoch <vsochat@stanford.edu>
@SuperKogito
Copy link
Copy Markdown
Member

Yeah you are right, my bad :/ I figured it is actually a bug and not something you did intentionally. That is why I committed directly to the master. But I still find the previous version a bit confusing so I will wrap my changes in a PR and we can discuss them?

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 5, 2020

Codecov Report

Merging #20 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #20   +/-   ##
=======================================
  Coverage   70.73%   70.73%           
=======================================
  Files          15       15           
  Lines         516      516           
=======================================
  Hits          365      365           
  Misses        151      151           
Impacted Files Coverage Δ
urlchecker/client/__init__.py 75.47% <ø> (ø)
urlchecker/version.py 100.00% <100.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 f43628f...75a97a8. Read the comment docs.

@vsoch
Copy link
Copy Markdown
Collaborator Author

vsoch commented Apr 5, 2020

We aren't changing the client to not have command line flags. Per my understanding you want to do:

urlchecker check . --cleanup true

Perhaps you aren't familiar with flags, but this is just totally wrong.

@vsoch
Copy link
Copy Markdown
Collaborator Author

vsoch commented Apr 5, 2020

The correct usage of a flag is:

urlchecker check . --cleanup

It's presence, period indicates true, "yes I want to cleanup."

@vsoch vsoch merged commit 3329014 into master Apr 5, 2020
@vsoch vsoch deleted the revert/back-to-working-client branch April 5, 2020 16:14
@SuperKogito
Copy link
Copy Markdown
Member

I see. I am sorry, it is my mistake. I was not aware of that :( I will revert the action changes then. and fix the latest release.

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