Clean Up Todos and Add in Issue Checker #640
Conversation
| @@ -169,7 +169,6 @@ func (p *Proposer) run(done <-chan struct{}, client pb.ProposerServiceClient) { | |||
|
|
|||
| bitmask := p.GenerateBitmask(p.pendingAttestation) | |||
|
|
|||
| // TODO(#552): Implement real proposals with randao reveals and attestation fields. | |||
There was a problem hiding this comment.
There is still a todo here, it's issue #619
There was a problem hiding this comment.
The script basically just checks using the github api if the issue is closed. If it is closed it throws an error. I am just removing todos that are referencing issues that are closed
There was a problem hiding this comment.
Np. I can add a TODO there later.
There was a problem hiding this comment.
I added it back with #619.
| @@ -76,8 +76,6 @@ func NewChainService(ctx context.Context, cfg *Config) (*ChainService, error) { | |||
|
|
|||
| // Start a blockchain service's main event loop. | |||
| func (c *ChainService) Start() { | |||
| // TODO(#474): Fetch the slot: (block, state) DAGs from persistent storage | |||
There was a problem hiding this comment.
Are we sure we don't need to do this anymore? @rauljordan can you confirm?
There was a problem hiding this comment.
I don't believe we need this as we already do everything using persistent storage. This seems outdated.
Codecov Report
@@ Coverage Diff @@
## master #640 +/- ##
==========================================
+ Coverage 71.35% 73.01% +1.66%
==========================================
Files 34 52 +18
Lines 2548 3476 +928
==========================================
+ Hits 1818 2538 +720
- Misses 569 716 +147
- Partials 161 222 +61
Continue to review full report at Codecov.
|
|
Workflow wise - is it just me or this feels weird?
It feels like the next person will be cleaning up for the last person quite often 😅 I feel everything should be contained within Bob. |
|
@terenc3t Yeah the workflow is not very smooth,but there is no way we can know if an issue will be closed before hand. The other alternative is that I remove the issue checker and we clean up manually |
|
Got it. We can give this a try. @prestonvanloon Do you have any thoughts for a tool like this? |
rauljordan
changed the title
|
I think this should be fine for some bookkeeping but agreed that making it a merge requirement could lead into weird situations where we might be fixing other people's left over todo's. We can decide to make it a build item if others agree, but we can just merge this in for now as it isn't modifying travis. |
| fi | ||
| done < <(grep -PrinH -o -h '(?<!context\.)todo\(#{0,1}\K(\d+)' --include \*.go *) |
There was a problem hiding this comment.
What is the purpose of this line? Wouldn't the script have exited on line 9 if there was any output of this command?
There was a problem hiding this comment.
The do-while loop, reads any line printed out by grep, which parses the issue numbers from the files. Without that line the loop would not do anything as there is nothing printed out
This cleans up Todos referencing closed issues and adds in an issue checker to script that checks the status of issues