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 Registering a Pool to stake pool documentation #1066

Merged
merged 6 commits into from
May 31, 2023

Conversation

sanskys
Copy link
Contributor

@sanskys sanskys commented May 19, 2023

Checklist

  • I have read the How to Contribute.
  • I have run yarn build after adding my changes without getting any errors.

Updating documentation or Bugfix

Replacing register-stake-pool-metadata.md with register-stake-pool.md

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Good to have these additions back in the staging workflow... but now more people will have to review them 🤓

@rphair rphair added the documentation Improvements or additions to documentation label May 23, 2023
@rphair rphair changed the title Adding register-stake-pool.md to operate a stake pool Add Registering a Pool to stake pool documentation May 23, 2023
@os11k
Copy link
Collaborator

os11k commented May 29, 2023

Sorry for nitpicking, but does TinyURL work okay with GitHub? Can we upload poolmetadata to GitHub and then shorten the URL with TinyURL? I thought when git.io stopped accepting new URLs, GitHub was no longer suitable for pool metadata.

Added info about case when we need to configure multiple relays
@rphair
Copy link
Collaborator

rphair commented May 29, 2023

@os11k I have no idea about the GitHub blocking the redirect URI since personally I would never use anything but a static URL for pool metadata. But I have seen it recommended in other Cardano setup guides & therefore haven't discriminated against it here. @sanskys can you verify that TinyURL in fact works with GitHub in your own testnet environment?

Also @os11k good idea to add the syntaxes in a2bd26e here... I remember needing to know that in the early days & having to confirm it on the Forum so it's good to have it all in one place.

@os11k
Copy link
Collaborator

os11k commented May 29, 2023

I'm thinking to add details how to create transaction with simple copy-paste commands(query UTXOs and etc), so we don't need to calculate fees as it is now in manually:

which in our case would be 9497237500 - 500000000 - 1000000 = 8996237500

Yes, seems tinyurl should work, as it is recommended by others...

minor change, updated description regarding multiple relays
@sanskys
Copy link
Contributor Author

sanskys commented May 31, 2023

@os11k I have no idea about the GitHub blocking the redirect URI since personally I would never use anything but a static URL for pool metadata. But I have seen it recommended in other Cardano setup guides & therefore haven't discriminated against it here. @sanskys can you verify that TinyURL in fact works with GitHub in your own testnet environment?

Also @os11k good idea to add the syntaxes in a2bd26e here... I remember needing to know that in the early days & having to confirm it on the Forum so it's good to have it all in one place.

tiny url works fine. Better would be obviously to store it where the url length is small

@rphair
Copy link
Collaborator

rphair commented May 31, 2023

@sanskys I assumed this was going into staging but I saw that we were missing our merge button... and in fact this PR is for merging into the main branch instead. Looks like it copied the target from your local fork where you were also working on main. I cannot be sure the merge would be a good thing even if we were allowed to do it.

@fill-the-fill @katomm I guess that means this PR has to be re-submitted? We have work from 3 different contributors in this PR so it would be nice if there were a way at the admin UI of flopping over the target from main to staging. 🤔

Or: I just made a current local copy of this @sanskys branch HEAD and can try making a new PR from that if @sanskys you would rather do it that way. If it's agreed to contain everybody's changes including @os11k then we can close this one.

@fill-the-fill
Copy link
Collaborator

@sanskys I assumed this was going into staging but I saw that we were missing our merge button... and in fact this PR is for merging into the main branch rather than staging. Looks like it copied the target from your local fork where you were also working on main. I cannot be sure the merge would be a good thing even if we were allowed to do it.

@fill-the-fill @katomm I guess that means this PR has to be re-submitted? We have work from 3 different contributors in this PR so it would be nice if there were a way at the admin UI of flopping over the target from main to staging. 🤔

Or: I just made a current local copy of this @sanskys branch HEAD and can try making a new PR from that if @sanskys you would rather do it that way. If it looks like it contains everybody's changes including @os11k then we can close this one.

Yeah, I've missed that part too..We can't merge it into main. You could potentially change the branch you would like to merge changes to though.

@rphair
Copy link
Collaborator

rphair commented May 31, 2023

change the branch you would like to merge changes to

@sanskys as the submitter I guess it is up to you to do that, since at the contributor level I don't have this option. 🤓 https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request

@rphair rphair changed the base branch from main to staging May 31, 2023 09:55
@rphair
Copy link
Collaborator

rphair commented May 31, 2023

OK, it let me do it in the heading of this PR (only submitter can change it from their own fork's UI I think). 😎

@rphair rphair merged commit 0cb8808 into cardano-foundation:staging May 31, 2023
1 check passed
@sanskys sanskys deleted the main branch June 2, 2023 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants