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

Boring utils dedup fix #978

Merged
merged 3 commits into from
Jan 22, 2019

Conversation

seldridge
Copy link
Member

Related issue:

Type of change: bug report

Impact: API modification

Development Phase: implementation

Release Notes

  • Fix BoringUtils.addSinks bug where sinks would still be
    deduplicated even if the user or BoringUtils.bore requested
    that they not be
  • Correct BoringUtils.{addSinks, addSources} parameter name from
    dedup to disableDedup

@seldridge seldridge requested a review from a team as a code owner January 9, 2019 05:15
Copy link
Contributor

@chick chick left a comment

Choose a reason for hiding this comment

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

This looks fine to me. Lot's of nice comments.

seldridge and others added 3 commits January 22, 2019 14:43
This fixes a bug where BoringUtils non-hierarchical sinks would be
deduplicated even when specified that they should not be.

h/t @ucbjrl for discovering this!

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This adds two tests to the BoringUtilsSpec to explicitly verify that
deduplication is required when boring. This adds tests that both
verify that the test passes as expected with deduplication enabled and
that the same test fails with deduplication disabled.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
This compresses the Scaladoc for BoringUtils slightly by using 120
character lines and removing unnecessary whitespace.

This also changes the poorly named "dedup" parameter to the what it
actually is: "disableDedup".

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Copy link
Contributor

@chick chick left a comment

Choose a reason for hiding this comment

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

lgtm

@seldridge seldridge merged commit eb6ddf4 into chipsalliance:master Jan 22, 2019
@seldridge seldridge deleted the boring-utils-dedup-fix branch January 22, 2019 20:11
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.

3 participants