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 10 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.

types `AccountInfo` and `u8`, respectively. A “slice” is a view into a block of
memory representing a contiguous sequence of elements of a single type, but
without needing to own the entire data. It’s important because it allows
functions to handle inputs of varying lengths efficiently.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good change. I think the old code was written by someone confused between vectors and slices. Probably me. 😅

Copy link
Author

Choose a reason for hiding this comment

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

alright. thanks for the review.

Comment on lines 83 to 84
`use` commands at the top of a Rust file to make the code more readable and
maintainable.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Honestly I think the old version was better. It's clear from 'we can bring an item into scope so that it can be reused throughout a file without specifying the full path each time.' that the code is now more readable.

Copy link
Author

Choose a reason for hiding this comment

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

Okay I would take this to the old version/way.

Comment on lines 311 to 315
account_info::AccountInfo, // Importing AccountInfo for account handling
entrypoint, // Importing entrypoint macro for defining program entry
entrypoint::ProgramResult, // Importing ProgramResult to handle function return type
pubkey::Pubkey, // Importing Pubkey to handle public keys
msg // Importing msg macro for logging messages
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make this less redundant? Just tell me what each of these are. You don't need to say their name.

account_info::AccountInfo, // A Solana account
entrypoint, // A macro to define the function that handles incoming instructions 

@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...

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