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

Add godox linter and resolve trivial TODO comments #1329

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

stana-miric
Copy link
Contributor

This PR includes:

  • Adding godox linter for detecting unresolved TODOs and FIXMEs
  • Resolving trivial TODOs
  • Marking TODOs which are going to be resolved in newly created tasks with nolint
    Tasks created for unsresolved TODOs are: EVM-519, EVM-521, EVM-522, EVM-523, EVM-524, EVM-525, EVM-527, EVM-528, EVM-529, EVM-540, EVM-541, EVM-542

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Additional comments

Note that ValidatorSetAddr and StateReceiverAddr are removed form PolyBFTConfig, since we have those addresses in the contracts package. As I discuss it with Stefan, it should not affect backward compatibility since they are not included in the blockchain

@stana-miric stana-miric self-assigned this Mar 23, 2023
@stana-miric stana-miric changed the title Added: godox linter,tasks for unresolved TODOs Add godox linter and resolve trievial TODO comments Mar 23, 2023
@stana-miric stana-miric force-pushed the EVM-443-remove-tod-os-and-fixm-es-from-the-code branch from ced36e9 to f7048d9 Compare March 23, 2023 10:50
@Stefan-Ethernal Stefan-Ethernal changed the title Add godox linter and resolve trievial TODO comments Add godox linter and resolve trivial TODO comments Mar 23, 2023
network/server.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

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

Generally LGTM, nice job!

@stana-miric stana-miric force-pushed the EVM-443-remove-tod-os-and-fixm-es-from-the-code branch from e721ab4 to 17429bb Compare March 24, 2023 08:48
@stana-miric stana-miric force-pushed the EVM-443-remove-tod-os-and-fixm-es-from-the-code branch from 17429bb to cf02fb4 Compare March 27, 2023 10:55
@stana-miric stana-miric merged commit 3b0e13b into develop Mar 27, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2023
@stana-miric stana-miric deleted the EVM-443-remove-tod-os-and-fixm-es-from-the-code branch March 27, 2023 12:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants