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.

Sign up for GitHub

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

Jump to bottom

[CHANGED] - Testnet Ticker format update <network name>ETH #856

Merged
merged 13 commits into from
Jul 7, 2022
Merged

[CHANGED] - Testnet Ticker format update <network name>ETH #856

merged 13 commits into from
Jul 7, 2022

Conversation

sethkfman
Copy link
Contributor

PR Title

  • Testnet Ticker update to include the ETH ticker prefixed with each test net.

Description

  • Added Testnet Object Type

  • Changed getTicker function to return prefixed ETH ticker

  • Added test case

  • CHANGED:

    • Updated the ticker symbol when displaying test net ETH it is now prefixed with the network (e.g. RopstenETH)

Checklist

  • Tests are included if applicable
  • Any added code is fully documented

@sethkfman sethkfman marked this pull request as ready for review June 17, 2022 21:10
@sethkfman sethkfman requested a review from a team as a code owner June 17, 2022 21:10
mcmire
mcmire reviewed Jun 22, 2022
View reviewed changes
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Hey @sethkfman! A couple of comments below.

src/constants.ts Outdated Show resolved Hide resolved
src/network/NetworkController.ts Outdated Show resolved Hide resolved
@sethkfman sethkfman requested a review from mcmire June 28, 2022 20:18
mcmire
mcmire approved these changes Jul 7, 2022
View reviewed changes
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to review this again. I think this branch needs another merge from main, but your update makes sense to me. Let me know if I need to re-approve this.

@sethkfman sethkfman merged commit fe771ee into main Jul 7, 2022
@sethkfman sethkfman deleted the improvement/testnet-ticker branch July 7, 2022 20:14
sethkfman added a commit that referenced this pull request Jul 7, 2022
@sethkfman
Revert "[CHANGED] - Testnet Ticker format update <network name>ETH (#856
8c6d216 
)"

This reverts commit fe771ee.
@sethkfman sethkfman mentioned this pull request Jul 7, 2022
Merged
sethkfman added a commit that referenced this pull request Jul 7, 2022
@sethkfman
Revert "[CHANGED] - Testnet Ticker format update <network name>ETH (#856
3df2f4c 
)" (#867)

This reverts commit fe771ee.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@sethkfman @MajorLift
[CHANGED] - Testnet Ticker format update <network name>ETH (#856)
b17ec08 
* added logic to update testnet ticker symbol

* added mainet ticker test

* added dist folder

* PR feedback added NetworkType constant and applied to NetworkController

* fixed lint warning
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@sethkfman @MajorLift
Revert "[CHANGED] - Testnet Ticker format update <network name>ETH (#856
36ab96d 
)" (#867)

This reverts commit fe771ee.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@sethkfman @MajorLift
[CHANGED] - Testnet Ticker format update <network name>ETH (#856)
b9f8068 
* added logic to update testnet ticker symbol

* added mainet ticker test

* added dist folder

* PR feedback added NetworkType constant and applied to NetworkController

* fixed lint warning
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@sethkfman @MajorLift
Revert "[CHANGED] - Testnet Ticker format update <network name>ETH (#856
33f38d6 
)" (#867)

This reverts commit fe771ee.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcmire mcmire mcmire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sethkfman @mcmire