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

Parse feature activation accounts #253

Closed

Conversation

ripatel-fd
Copy link

@ripatel-fd ripatel-fd commented Jun 2, 2023

@vercel
Copy link

vercel bot commented Jun 2, 2023

@terorie is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@ripatel-fd
Copy link
Author

I'm not good at frontend, so if this PR needs changes, I suggest a maintainer close this PR and recreate it to save time. Many thanks!

</tr>
);
}
console.log(feature);

Choose a reason for hiding this comment

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

is this intentionally left?

@vercel
Copy link

vercel bot commented Jun 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 2, 2023 1:29pm

@ngundotra
Copy link
Collaborator

Can you please follow the format for PRs + issues next time? @ripatel-jump

Just add a screenshot + a link or two to test the feature you've added would help

@ngundotra
Copy link
Collaborator

Why did you close this PR?

@ripatel-fd
Copy link
Author

@ngundotra I meant to submit a draft PR, but I mostly just wanted to provide a reference implementation of how this feature could be implemented and save the round trips of review. Would your team be able to take ownership of this and get it to main? Otherwise, will reopen and get it through, and also add human-readable descriptions of the feature accounts.

@steveluscher
Copy link
Collaborator

Thanks for this! I'm concerned to the degree that this UI would only work in two of these three cases:

  1. ✅ When a feature has been activated at the epoch boundary for which it was scheduled, we can read out the activation timestamp and render it in the UI
  2. ✅ When a feature has been scheduled for activation but the activation epoch boundary hasn't been reached, the timestamp is none and we can render ‘not active’ in the UI
  3. ❌ When a feature has not been scheduled at all, its feature flag account doesn't exist so there's no UI we can show one way or another

I worry that this will confuse folks coming to the Explorer in case 3, expecting to see information about a feature, and seeing nothing instead.

steveluscher added a commit that referenced this pull request Jun 2, 2023
…tivated (#255)

This implements:

* When a feature has been activated at the epoch boundary for which it
was scheduled, we can read out the activation slot and render it in the
UI.
* When a feature has been scheduled for activation but the activation
epoch boundary hasn't been reached, the slot is `none` and we can render
‘not active’ in the UI.

This does not implement:
* When a feature has not been scheduled at all. Its feature flag account
doesn't exist so there's no UI we can show one way or another.

_NOTE: Originally made by @terorie as PR #253. I was not able to push to
that branch, so had to open a new PR._

---------

Co-authored-by: Richard Patel <ripatel@jumptrading.com>
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.

5 participants