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

Update docs #2083

Merged
merged 7 commits into from Dec 1, 2022
Merged

Update docs #2083

merged 7 commits into from Dec 1, 2022

Conversation

plowsof
Copy link
Contributor

@plowsof plowsof commented Oct 19, 2022

closes #2022
closes #2026
closes #1842
closes #1528
closes #2075
closes #2084
closes #2021
closes #2089
closes #2093
closes #2098

Note: set_log_level / categories are not documented in wallet rpc - but they are in daemon rpc -i think it would be best to list the commands in wallet rpc index but simply link people to the description like this , then if something is updated, it only has to be set in one place.
add_aux_pow is a wip but should not block this PR

@netlify
Copy link

netlify bot commented Oct 19, 2022

Deploy Preview for barolo-time-757cf9 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ecc517e
🔍 Latest deploy log https://app.netlify.com/sites/barolo-time-757cf9/deploys/637961d1205ab3000909dc46
😎 Deploy Preview https://deploy-preview-2083--barolo-time-757cf9.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@erciccione
Copy link
Contributor

Thanks fro transferring to a single PR. The normal approach in this case is to squash but from the other conversation i take you prefer to have multiple commits for these changes. In past we usually squashed multiple commits changing a single file, but if he luigi is ok with multiple commits, ok for me.

wallet rpc has no space, but my additions do

I would keep the new changes consistent with the already existent structure.

@plowsof plowsof force-pushed the update_docs branch 2 times, most recently from 1ed20bd to 294b54d Compare November 19, 2022 04:39
Copy link
Contributor

@erciccione erciccione left a comment

Choose a reason for hiding this comment

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

@plowsof I would freeze the number of issues that will be solved in this PR, so that it can be reviewed and finally merged. I think it's getting to a state where more and more stuff is added here when new issues are opened, but that will result in the pr never getting merged and constant re-reviewing.

I suggest to not add any more commits to this pr and to have it reviewed. if more issues arise related to these docs, a new WIP pr can be opened and based on this one. I would keep as a general rule that if a dense PR is already reviewed/approved, better leave it as it is and open a new PR for additional commits.

Copy link
Contributor

@selsta selsta left a comment

Choose a reason for hiding this comment

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

All other changes will go in a separate PR.

@erciccione erciccione moved this from Review Needed to Ready for merge in monero-site: PR workflow Nov 24, 2022
@luigi1111 luigi1111 merged commit 7315745 into monero-project:master Dec 1, 2022
monero-site: PR workflow automation moved this from Ready for merge to Done Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment