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

refactor: RepresentativePowers to StakedTokens #1333

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

insumity
Copy link
Contributor

@insumity insumity commented Sep 27, 2023

Description

This is a minor-refactor PR and hence no issue is created for it.

Based on @p-offtermatt comment:

Seems ok to me. Could rename it to something like StakedTokens instead of RepresentativePowers (looking at the code, I think that's what this actually is, right?)

We rename RepresentativePowers to StakedTokens. We can see that StakedTokens is a better name if we look at what getRepresentativePower is computing, which is just the number of tokens:

func (tr TestConfig) getRepresentativePower(chain ChainID, validator ValidatorID) uint {
	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
	bz, err := exec.Command("docker", "exec", tr.containerConfig.InstanceName, tr.chainConfigs[chain].BinaryName,

		"query", "staking", "validator",
		tr.validatorConfigs[validator].ValoperAddress,
		`--node`, tr.getQueryNode(chain),
		`-o`, `json`,
	).CombinedOutput()
	if err != nil {
		log.Fatal(err, "\n", string(bz))
	}
	amount := gjson.Get(string(bz), `tokens`)
	return uint(amount.Uint())
}

@github-actions github-actions bot added the C:Testing Assigned automatically by the PR labeler label Sep 27, 2023
@insumity insumity changed the title refactor RepresentativePowers to StakedTokens minor: refactor RepresentativePowers to StakedTokens Sep 27, 2023
@insumity insumity marked this pull request as ready for review September 27, 2023 10:45
@insumity insumity requested a review from a team as a code owner September 27, 2023 10:45
@insumity insumity changed the title minor: refactor RepresentativePowers to StakedTokens minor refactor: RepresentativePowers to StakedTokens Sep 27, 2023
Copy link
Contributor

@p-offtermatt p-offtermatt left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the refactor! 🥇 just a nit, the prefix for the pr should probably just be refactor
https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json
not a big deal but this ensures that this is e.g. easily evaluated via machine

@insumity insumity changed the title minor refactor: RepresentativePowers to StakedTokens refactor: RepresentativePowers to StakedTokens Sep 27, 2023
@insumity insumity merged commit 3cbf9c8 into main Sep 27, 2023
14 checks passed
@insumity insumity deleted the insumity/refactor-representative-powers branch September 27, 2023 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Testing Assigned automatically by the PR labeler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants