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

refactor: break down the cli application function to specific file #74

Conversation

iamnamananand996
Copy link
Contributor

@iamnamananand996 iamnamananand996 commented Aug 9, 2024

Status Type ⚠️ Core Change Issue
Ready Refactor Yes

Problem

Right now, if we observe the cli-application it is centralized into 2 big files, which in a long term will become very hard to mange, better to break down these file to specific task files.

Solution

  • breakdown lib.rs file
  • breakdown rust_template.rs
  • rather than hardcoding the template text into code directly, add it into a text file and read it.

@iamnamananand996 iamnamananand996 marked this pull request as draft August 9, 2024 14:11
@iamnamananand996
Copy link
Contributor Author

iamnamananand996 commented Aug 14, 2024

Hi @GabrielePicco,

for part 3

  • rather than hardcoding the template text into code directly, add it into a text file and read it.

I came up of the following solution.
Just, want to check with, if you liked it or not then only will procced with this changes for whole cli application.

Solution

  • folder structure

Screenshot 2024-08-14 205210

  • component_template.txt
use bolt_lang::*;

declare_id!("{{PROGRAM_ID}}");

#[component]
#[derive(Default)]
pub struct {{STRUCT_NAME}} {
    pub x: i64,
    pub y: i64,
    pub z: i64,
    #[max_len(20)]
    pub description: String,
}

In the above text file, we need observe the way the variable name are defined. {{PROGRAM_ID}} and {{STRUCT_NAME}}
and do find and replace in the create_component_template_simple function below, before procced this, we need to agree on, either we liked this way for definition or not.

  • create_component_template_simple function
/// Create a component which holds position data.
fn create_component_template_simple(name: &str, program_path: &Path) -> Files {
    // Define the path to the template file in the "templates" folder
    let template_path = Path::new("templates").join("component_template.txt");

    // Read the template content from the text file
    let mut template =
        fs::read_to_string(&template_path).expect("Failed to read component template file");

    let program_id = anchor_cli::rust_template::get_or_create_program_id(name).to_string();

    // Replace placeholders with actual values
    template = template.replace("{{PROGRAM_ID}}", &program_id);
    template = template.replace("{{STRUCT_NAME}}", &name.to_upper_camel_case());
    // Return the modified string as part of the function
    vec![(program_path.join("src").join("lib.rs"), template)]
}

@GabrielePicco
Copy link
Contributor

Thanks @iamnamananand996 for the contribution. Separating and simplifying the lib.rs and rust_template.rs sounds like a good idea. The template system and reading from a .txt add complexity, I would rather keep it programatic inside the rust code

@iamnamananand996
Copy link
Contributor Author

iamnamananand996 commented Aug 17, 2024

Hi @GabrielePicco, Thanks for the suggestion above, really appreciate it.

I dig down into the test case failure also, it clearer show that outside contributor are not able to read the deployment keys




BTW, above PR is ready now for review.

@iamnamananand996 iamnamananand996 marked this pull request as ready for review August 17, 2024 11:28
@GabrielePicco
Copy link
Contributor

@iamnamananand996 the deployment keys should be injected from secret ENV variables by the workflow: https://github.com/magicblock-labs/bolt/blob/main/.github/workflows/run-tests.yml#L142

@iamnamananand996
Copy link
Contributor Author

Hi @GabrielePicco, you are correct, deployment keys should be injected from secret ENV variables by the workflow.

Problem is by default GitHub does not allow it for Fork repository to read the secret ENV variables.
To fix this Created a PR, #76 this will fix the problem for reading the secret ENV variables

@GabrielePicco
Copy link
Contributor

Thanks @iamnamananand996. #76 is now merged, rebasing the branch on mainshould eventually solve the CI/CD issue

@iamnamananand996 iamnamananand996 force-pushed the maintain-cli-application-fuction branch from 96c421c to afe4215 Compare August 27, 2024 10:04
@iamnamananand996
Copy link
Contributor Author

Hi @GabrielePicco, this PR is hanging here from a long time.

can we merge this, if it looks good to you.
Please let me know, if you think we need any further changes, happy to contribute more.

@GabrielePicco GabrielePicco merged commit 9090a9e into magicblock-labs:main Sep 15, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants