-
Notifications
You must be signed in to change notification settings - Fork 1k
[Contribute] - CBOR debugging + Sidebar Structure + Governance #1636
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
base: staging
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also looks like a big format tidying. 😊 @0xBora are the commits done as far as you know & now ready for review?
Hey @rphair, yeah I think I ll stop here with this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked in local build ✅ particularly on the sidebar changes and everything seems very well placed now.
- The new CBOR page is great & I had no idea anything like that existed (I'm not at the level to use it, nor to give feedback on the particulars, but I know it will be a help to others).
- It never occurred to me before that Funding would be so much better placed "above the fold" where potential & current migrants to Cardano can find it in the first minute. Strategically this is a great improvement as well as a practical help to newcomers to the Cardano community.
From the white-space-only changes I have inferred a new coding standard of "no white space at the end of a line". We've had some PRs before which eliminate white space at the end of lines outside the scope of a PR: mainly removed incidentally by the contributor's Markdown editor.
- I wonder if I/we should keep asking contributors to just focus their PRs on the "real" changes without those incidental cleanups... or if they are always considered to be an improvement (as I believe they are in this PR: especially since so much of it is being cleaned up at once along with other formatting corrections). 🤔
Co-authored-by: Robert Phair <[email protected]>
Co-authored-by: Robert Phair <[email protected]>
Co-authored-by: Robert Phair <[email protected]>
Thanks for the great catches @rphair regarding the quoting etc. Maybe to keep the focus of PRs more clear without adding in formatting corrections via markdown linters can be in dedicated PRs where there are no content changes and it can be ran every once in a while. As you yourself are quite experienced in managing the structure of such things in all the years of editoring/reviewing, I would be more than happy to follow the lead here. The formatting fixes in this PR is just me using the suggested markdown linter when moving stuff around to be honest. |
Another note regarding mermaid diagrams: This PR also introduces another image for a transaction which could have been handled by mermaid graphs as discussed in the previous PR that got merged, and for these cases I think in the coming times I will do a rerun of the portal to replace them with proper graphs when possible. In the current phase where I am making a lot of these changes, I just did not invest the time for them yet. Will be handled in the future though. |
I like the idea of keeping these 2 things in separate PRs from any content-changing PR:
Since we already have a reason to break "rule" # 1 here (especially the superfluous punctuation at the end of many headers) it does make sense here (and in similar PRs with global scope) to also do the white space corrections. 😎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tagging other reviewers in hope of expediting this because of the huge potential for merge conflict otherwise.
Checklist
yarn build
after adding my changes without getting any errors.yarn.lock
(or have removed these changes).Updating documentation or Bugfix
Introduces how to debug CBOR as an advanced topic + moves funding page to Get Started section out of Governance