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] Split up exec_pipeline() for readability #354

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

itislu
Copy link
Collaborator

@itislu itislu commented Jul 4, 2024

The function was very dense and we resorted to bad practices like the comma operator to keep the function length within 25 lines for Norminette.

  • Split up long else block of exec_pipeline() into separate exec_cmd_table() function and rename the original function to iter_pipeline().
  • Change unnecessary long while (check cmd_table_type) loop check to while (true) + if (check cmd_table_type) + break.
  • Remove comma operator.

@itislu itislu added the chore A routine task label Jul 4, 2024
@@ -46,35 +50,44 @@ void fork_pipeline(t_sh *shell, t_list_d **cmd_table_node)
}
}

static void exec_pipeline(t_sh *shell, t_list_d **cmd_table_node)
static void iter_pipeline(t_sh *shell, t_list_d **cmd_table_node)
Copy link
Collaborator Author

@itislu itislu Jul 4, 2024

Choose a reason for hiding this comment

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

Is the new name of this function okay for you?

@itislu itislu force-pushed the refactor-pipeline-readability branch 3 times, most recently from 73daa62 to 90e2af8 Compare July 10, 2024 21:47
@itislu itislu force-pushed the refactor-pipeline-readability branch 2 times, most recently from e539ab2 to c093097 Compare July 15, 2024 03:13
@itislu itislu force-pushed the refactor-pipeline-readability branch from c093097 to 8aa7b79 Compare September 14, 2024 17:23
@itislu itislu requested a review from LeaYeh September 14, 2024 17:32
@itislu itislu force-pushed the refactor-pipeline-readability branch from 8aa7b79 to 2a824c4 Compare September 27, 2024 01:21
The function was very dense and we resorted to bad practices like the comma operator to keep the function length within 25 lines for Norminette.
@LeaYeh LeaYeh merged commit 1cff8bf into main Nov 14, 2024
41 checks passed
@LeaYeh LeaYeh deleted the refactor-pipeline-readability branch November 14, 2024 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A routine task
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants