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

Grammatic error fix, Comments in code for easy understanding, more explanation on msg macro #417

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

NtemKenyor
Copy link

Problem

The original lesson/code provided lacked clarity in certain areas, particularly in the explanation of paths and scope, function declarations, the concept of Solana programs, entry point syntax, and error handling. These issues might lead to confusion for readers, especially those new to Rust or Solana programming.

Summary of Changes

  1. Paths and Scope Clarification:

    • Expanded the explanation of the use statement, highlighting its importance in making code more readable and maintainable by bringing items into scope.
  2. Function Declaration Explanation:

    • Added a more detailed explanation regarding references and slices in function arguments, emphasizing the significance of slices in handling inputs of varying lengths efficiently.
  3. Entry Point Code - Syntax and Comments:

    • Revised the entry point code to ensure accuracy and added detailed comments to enhance understanding. This includes explanations of the imported modules and the purpose of each section of the code.
  4. msg! Macro and Error Handling:

    • Provided additional information on the usage of the msg! macro for logging and introduced basic concepts of error handling within Solana programs.

Copy link
Collaborator

@mikemaccana mikemaccana left a comment

Choose a reason for hiding this comment

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

I reviewed this a while ago but forgot to click the button. Please see the comments above! As someone else's changes were merged you will also need to resolve some conflicts.

@NtemKenyor
Copy link
Author

I reviewed this a while ago but forgot to click the button. Please see the comments above! As someone else's changes were merged you will also need to resolve some conflicts.

okay I am on it.

@NtemKenyor
Copy link
Author

@mikemaccana thank you for making out some time to review this work. please if you have any more changes you would love me to implement, I would be glad to make the fix.

I also messaged you on Telegram to find out if we can add more language support to "intro-to-solana".

Reverted changes on arbitrary-cpi taking it back to normal.
@NtemKenyor
Copy link
Author

NtemKenyor commented Sep 20, 2024

Hello @mikemaccana and @nickfrosty

Hope you're both doing great 👍. I have made the necessary adjustments and I also updated arbitrary-cpi.md to it's most recent form to avoid any conflicts. Pls 🙏 merge if it's ok. Thank you and best regards...

@NtemKenyor
Copy link
Author

Hello @mikemaccana and @nickfrosty

Hope you're both doing great 👍. Updated my Pull request so it's updated and I have only focused on the required file: content/courses/native-onchain-development/hello-world-program.md

Please 🙏 help review as to merge. I understand that reviewing may be stressful - I'm grateful.

Copy link
Author

@NtemKenyor NtemKenyor left a comment

Choose a reason for hiding this comment

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

I Focused on the reviewed section alone. If there are any other changes you would love me to make, please let me know.

Copy link
Collaborator

@mikemaccana mikemaccana left a comment

Choose a reason for hiding this comment

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

Minor fix but ready to go afrerwards.

@NtemKenyor
Copy link
Author

@mikemaccana , @nickfrosty please don't forget to include my contribution.

Hello Bro 👋 hope you're doing good? I'm sorry for bordering/disturbing you but I just felt it's been a while. Pls help merge this pull request or give me further advise. Thank you very much for your understanding.

I'm grateful 🥰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants