Opened 2 months ago

Closed 4 weeks ago

#28602 closed defect (fixed)

The call to generate the bw file is passing an old argument

Reported by: juga Owned by: juga
Priority: Medium Milestone: sbws: 1.0.x-final
Component: Core Tor/sbws Version:
Severity: Normal Keywords: sbws-1.0-must-moved-20181128
Cc: juga Actual Points:
Parent ID: #25925 Points:
Reviewer: teor Sponsor:

Description

The generate cli argument changed name to round_digs but the argument passed to V3BWFile.from_results is still using torflow_round_digs.

Child Tickets

Change History (15)

comment:1 Changed 2 months ago by juga

Status: assignedneeds_review

comment:2 Changed 8 weeks ago by teor

Status: needs_reviewneeds_revision

Removing a command-line argument is a breaking change.

Please keep the old argument for compatibility.
(Or increment the sbws version to 2.0.)

comment:3 Changed 8 weeks ago by teor

Keywords: sbws-1.0-must-moved-20181128 added
Milestone: sbws 1.0 (MVP must)sbws 1.0.3

Moving all sbws 1.0 must bugs to 1.0.3.

comment:4 Changed 8 weeks ago by teor

Milestone: sbws 1.0.3sbws 1.0.x

Milestone renamed

comment:5 Changed 8 weeks ago by teor

Milestone: sbws 1.0.xsbws: 1.0.x

Milestone renamed

comment:6 Changed 8 weeks ago by teor

Milestone: sbws: 1.0.xsbws: 1.0.x-final

Milestone renamed

comment:7 in reply to:  2 Changed 7 weeks ago by juga

Status: needs_revisionneeds_review

Replying to teor:

Removing a command-line argument is a breaking change.

Please keep the old argument for compatibility.
(Or increment the sbws version to 2.0.)

right, sorry forgot. Changed PR to keep the old argument.

comment:8 Changed 7 weeks ago by teor

Status: needs_reviewneeds_revision

This patch changes the meaning of --torflow-round-digs, which is also a breaking change.

When you make changes after a review, please keep the same branch, and use fixup commits.
Then the reviewer can just review the new changes.

comment:9 in reply to:  8 ; Changed 7 weeks ago by juga

Replying to teor:

This patch changes the meaning of --torflow-round-digs, which is also a breaking change.

Assuming that by default we want to use --round-digs (unless --torflow-round-digs is provide), what do you suggest?, use that option when it's provided and use --round-digs by default?, should --round-digs still exist?.

When you make changes after a review, please keep the same branch, and use fixup commits.
Then the reviewer can just review the new changes.

Sorry i didn't realize it was reviewed, interpreted it as a comment.

comment:10 Changed 7 weeks ago by teor

Reviewer: teor

comment:11 in reply to:  9 Changed 7 weeks ago by teor

Replying to juga:

Replying to teor:

This patch changes the meaning of --torflow-round-digs, which is also a breaking change.

Assuming that by default we want to use --round-digs (unless --torflow-round-digs is provide), what do you suggest?, use that option when it's provided and use --round-digs by default?, should --round-digs still exist?.

If -r, --round-digs, and --torflow-round-digs are different names for the same argument, then sbws will behave the same when --torflow-round-digs is used. I will make a suggestion on the pull request.

comment:12 Changed 5 weeks ago by juga

Status: needs_revisionneeds_review

I think i fixed now. Left comments in the PR

comment:13 Changed 5 weeks ago by juga

I should change the commit message of the first one when i'll squash it.

comment:14 Changed 4 weeks ago by teor

Status: needs_reviewmerge_ready

Looks good to me. Thanks!

comment:15 Changed 4 weeks ago by juga

Resolution: fixed
Status: merge_readyclosed

Merged

Note: See TracTickets for help on using tickets.