-
Notifications
You must be signed in to change notification settings - Fork 295
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
docs(examples/discounted-asset-trade): fix rendering of docs on website #3301
Conversation
@petermetz Does this look fine or should I make any further changes in this ? |
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.
@rajat-dlt Thank you very much for this improvement! LGTM with comments (nit-picks):
- It's hard to untangle the actual changes from the white-space changes, could you please remove the white-space changes from the diff?
If there are formatting issues it's OK to submit a separate pull request that includes only white-space and formatting changes but not logical changes. It's just not good to mix the two because then the review becomes much harder.
For last, if the white-space changes are the fix then it's OK, but please let me know just to be sure. - Please change the commit message and the PR title to something that doesn't qualify this as a production bug (the
fix
scope is reserved for problems that are affecting the software running in production). So my recommendation is to change it todocs(examples/discounted-asset-trade): fix rendering of docs on website
@rajat-dlt Also, please use prettier to format changed file according to root config |
23b621d
to
4239b6c
Compare
Let me know if any further changes are required. |
4239b6c
to
f03c82b
Compare
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.
@rajat-dlt We are getting closer but there are still some things:
- You've pushed a second commit, please squash them together so that the pull request contains only a single commit.
- You haven't amended the git commit message, just edited the GitHub PR title but they are separate things:
If you need any help with the git mechanics you can also join our daily pair programming call on the Hyperledger Discord server that runs at 10 AM, Pacific Time on weekdays: https://wiki.hyperledger.org/display/cactus/Daily+Pair+Programming+Calls
There you can share your screen if necessary and I can help you with the details!
04d0d41
to
5bfe727
Compare
@petermetz, I was expecting from the portal all the commits that are made and squashed and merged into 1 commit. Thus I improved the description here and then went on to make improvements by making multiple commits in the same PR. Apologies for the oversight. |
@rajat-dlt No worries, glad I could help! 2 things:
|
f51b336
to
34eed2a
Compare
79924b6
to
ffaf47f
Compare
@petermetz , |
@outSH @petermetz , |
@rajat-dlt All good from my side, we're waiting for Peter approval :) |
@rajat-dlt Correct. We try to review everything as soon as possible, but it can easily take a few days. With that said, it's looking good now, thank you for the contribution! |
Signed-off-by: Rajat Sharma <rajat16.sharma@ril.com>
ffaf47f
to
406bdfa
Compare
Closes issue #3335 . |
fix: Documentation fixed for cactus-example-discounted-asset-trade
At present, the documentation on the GitHub page isn't rendering correctly, and there are issues with numbering. Therefore, adjustments have been made to rectify these issues and ensure that the documentation functions as intended.
Signed-off-by: Rajat Sharma rajat16.sharma@ril.com
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.