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

Redesign of the nurse command #183

Merged
merged 1 commit into from
Jul 30, 2023
Merged

Redesign of the nurse command #183

merged 1 commit into from
Jul 30, 2023

Conversation

vincenzopalazzo
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo commented Jul 29, 2023

Please review each commit by review also the comments of the module.

@vincenzopalazzo vincenzopalazzo added the ❤️ - RFC Request for Comments, it is under consideration label Jul 29, 2023
@vincenzopalazzo vincenzopalazzo self-assigned this Jul 29, 2023
@netlify
Copy link

netlify bot commented Jul 29, 2023

Deploy Preview for coffee-docs canceled.

Name Link
🔨 Latest commit bad8427
🔍 Latest deploy log https://app.netlify.com/sites/coffee-docs/deploys/64c5674b69c5d8000830d2ed

Copy link
Collaborator

@tareknaser tareknaser left a comment

Choose a reason for hiding this comment

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

These are just typos
The skeleton is very nice
I can work on this as well

Comment on lines +6 to +21
//! >The basic idea behind the Strategy pattern is that, given an
//! > algorithm solving a particular problem, we define only
//! > the skeleton of the algorithm at an abstract level, and we
//! > separate the specific algorithm’s implementation into
//! > different parts.
//! >
//! > In this way, a client using the algorithm may choose
//! > a specific implementation, while the general algorithm
//! > workflow remains the same. In other words, the abstract
//! > specification of the class does not depend on the specific
//! > implementation of the derived class, but specific implementation
//! > must adhere to the abstract specification.
//!
//! So in this specific case the nurse command may need
//! different kind of recovery algorithm, so we should
//! be able to choose the algorithm at runtime.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the explanation

coffee_core/src/nurse/chain.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/chain.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/chain.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/chain.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/chain.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/chain.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/strategy.rs Outdated Show resolved Hide resolved
coffee_core/src/nurse/strategy.rs Outdated Show resolved Hide resolved
@vincenzopalazzo
Copy link
Contributor Author

I can work on this as well

Yeah I will leave this to you now, let me fix the review and we merge it as a draft

@vincenzopalazzo vincenzopalazzo force-pushed the macros/nurse-logic branch 2 times, most recently from 0be7404 to 44090d1 Compare July 29, 2023 19:22
@vincenzopalazzo vincenzopalazzo marked this pull request as ready for review July 29, 2023 19:23
This commit introduce a basic architecture to run the nurse command
by using two design pattern, that are the chain of responsability
for choosing what kind of strategy apply, and a strategy pattern
to abstract the algorithm under a basic view.

Signed-off-by: Vincenzo Palazzo <[email protected]>

feat(core): add nurse chain of responsability to select the recovery algorithm

Signed-off-by: Vincenzo Palazzo <[email protected]>
@vincenzopalazzo
Copy link
Contributor Author

vincenzopalazzo commented Jul 30, 2023

Merging this and leaving it to play with to @tareknaser360

@vincenzopalazzo vincenzopalazzo merged commit 7991236 into master Jul 30, 2023
9 checks passed
@vincenzopalazzo vincenzopalazzo deleted the macros/nurse-logic branch July 30, 2023 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❤️ - RFC Request for Comments, it is under consideration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants