chipsalliance / chipsalliance/rocket-chip

Possible comment errors(confusion at least)

Open
#2,783 0 comments 0 reactions 0 assignees View on GitHub
question
Dominant language
Scala
Stars
3.9k
Forks
1.3k
Avg merge
5d 13m
Merged PRs (30d)
1

Description

https://github.com/chipsalliance/rocket-chip/blob/46c88b71056581a6bf2a0c4febd5ec3d768c6c59/src/main/scala/diplomacy/Nodes.scala#L995
The comment indicates allSink means `The total number of :*= operators where we're on the left.` I think it is very confusing using 'we' here. From my understanding, allSink means the total number of sinkCard of the nodes in a node island connected by the flex operator(:*=*), when resolving the flex operator, we check the :*= or :=* operators attached to the whole island, allSink counts all the :*= operators connected to the island, and allSource counts all the :=* operators connected to the island. If my guess is right, then using 'we' here seems a little bit confusing, when I first saw the comment, I thought allSink means `the total number of :*= operators connected to this island, and which the island itself is located at the lhs`. Can anyone clarify if my understanding is right? Thanks!

Contributor guide

Open the contributing guide

Research direction

Open src/main/scala/diplomacy/Nodes.scala at line 995 and inspect the surrounding allSink, allSource, and flex-operator resolution logic. Confirm what the comment describes and whether “we” creates ambiguity; done means the comment clearly states the relevant operators and node-island relationship.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala
Domain
documentation
Issue type
Documentation
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.