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

services/horizon: Rename OperationFeeStats to FeeStats #1953

Merged

Conversation

shivamdixit
Copy link
Contributor

What

Renames OperationFeeStats to FeeStats across the horizon service.

Why

Fixes #1952

Known limitations

N/A

@cla-bot
Copy link

cla-bot bot commented Nov 19, 2019

Thank you for your pull request and welcome to our community! We require contributors to sign a Contributor License Agreement before a change can be accepted and it doesn't look like we have @shivamdixit on file as having signed it. Please follow the link to sign and let us know when you've signed.

Project Maintainers: After @shivamdixit have let us know they've signed the CLA, check the Google Form Sheet, update the clabot-config with their GitHub Username, then update the PR by commenting with @cla-bot check. If this PR has been opened by a member of the @stellar org, they need adding to the clabot-config list too.

@shivamdixit
Copy link
Contributor Author

@cla-bot check

@cla-bot

This comment has been minimized.

@cla-bot

This comment has been minimized.

@shivamdixit
Copy link
Contributor Author

@cla-bot check

@cla-bot

This comment has been minimized.

@cla-bot

This comment has been minimized.

@leighmcculloch
Copy link
Member

@shivamdixit It's a manual process to update @cla-bot after you've filled out the Google Form. The check will result in the same message until that's done.

I've just taken a look and noted you didn't complete the Mailing Address form. Are you unable to provide that required field on the CLA? @ire-and-curses Do we need this field completed?

@leighmcculloch
Copy link
Member

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Nov 19, 2019
@cla-bot
Copy link

cla-bot bot commented Nov 19, 2019

The cla-bot has been summoned, and re-checked this pull request!

@abuiles abuiles changed the base branch from master to release-horizon-v0.24.0 November 20, 2019 00:36
Copy link
Contributor

@abuiles abuiles left a comment

Choose a reason for hiding this comment

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

@shivamdixit thanks!

BTW, I updated the branch to release-horizon-v0.24.0, we don't follow trunk based development but something similar to gitflow, where we have a release branch where we add all the new changes for the upcoming version and then merge back to master.

So for your next contribution :) you can branch off from release-horizon-vX.X.X and avoid possible conflicts.

Copy link
Contributor

@abuiles abuiles left a comment

Choose a reason for hiding this comment

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

@shivamdixit sorry, can you actually update the commits so the documentation one is not included (2c65553)?

I think you could do something like

git reset --hard release-horizon-v0.24.0
git cherry-pick 3f64d3c21573b1878fdf7a8d8eaf8519347dcc1a
git push -f

@ire-and-curses
Copy link
Member

@shivamdixit It's a manual process to update @cla-bot after you've filled out the Google Form. The check will result in the same message until that's done.

I've just taken a look and noted you didn't complete the Mailing Address form. Are you unable to provide that required field on the CLA? @ire-and-curses Do we need this field completed?

No need. We should just remove that field IMO.

@abuiles abuiles force-pushed the rename-OperationFeeStats branch from 3f64d3c to e85b02a Compare November 21, 2019 18:05
Copy link
Contributor

@abuiles abuiles left a comment

Choose a reason for hiding this comment

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

@shivamdixit I did the fix in your branch, merging this, thanks!

@abuiles abuiles merged commit 3c69dd8 into stellar:release-horizon-v0.24.0 Nov 21, 2019
@shivamdixit
Copy link
Contributor Author

I've just taken a look and noted you didn't complete the Mailing Address form. Are you unable to provide that required field on the CLA? @ire-and-curses Do we need this field completed?

@leighmcculloch To be honest, I don't see a reason to provide Mailing Address for signing CLA form. For the privacy reasons I prefer not to provide that.

So for your next contribution :) you can branch off from release-horizon-vX.X.X and avoid possible conflicts.

@abuiles Thanks! noted.

@shivamdixit I did the fix in your branch, merging this, thanks!

Sorry for late response and thanks for fixing it. Next time I will be more active after sending out a fix.

@abuiles
Copy link
Contributor

abuiles commented Nov 25, 2019

@shivamdixit thanks!

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.

4 participants