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

Rewrite GoldMiner main entrypoint #43

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Conversation

MatheusRich
Copy link
Contributor

Previously, the GoldMiner module (now a class) had methods for each part of the mining process: exploration, smithing, and distribution.

The orchestration of when and how these methods were called was done in the exe/gold_miner script. This wasn't ideal since there's no way to guarantee the order in which the methods are called, and a script is arguably not the best place to put this logic. Furthermore, that arrangement wasn't flexible, as there was no way to specify different explorers, smiths, or distributors.

This commit rewrites the GoldMiner class to have an initialize method that receives the explorer, smith, and distributor as arguments. It also adds a #mine method that orchestrates the mining process.


puts "[DEBUG] #{msg}"
def prepare
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A sub-goal of mine with this rewrite was to remove logic from this file. While I think the current approach is much cleaner than before, this method is still something I want to improve later.

Comment on lines +17 to +20
.bind { GoldMiner::Slack::Client.build(api_token: ENV["SLACK_API_TOKEN"]) }
.fmap { |slack_client|
GoldMiner::SlackExplorer.new(slack_client, GoldMiner::AuthorConfig.default)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we could have a SlackExplorer.build method that tries to by the client first and uses default values so we could rewrite this in a single line?

Suggested change
.bind { GoldMiner::Slack::Client.build(api_token: ENV["SLACK_API_TOKEN"]) }
.fmap { |slack_client|
GoldMiner::SlackExplorer.new(slack_client, GoldMiner::AuthorConfig.default)
}
.bind { GoldMiner::SlackExplorer.build }

Previously, the `GoldMiner` module (now a class) had methods for each part of
the mining process: exploration, smithing, and distribution.

The orchestration of when and how these methods were called was done in the
`exe/gold_miner` script. This wasn't ideal since there's no way to guarantee the
order in which the methods are called, and a script is arguably not the best
place to put this logic. Furthermore, that arrangement wasn't flexible, as there
was no way to specify different explorers, smiths, or distributors.

This commit rewrites the `GoldMiner` class to have an `initialize` method that
receives the explorer, smith, and distributor as arguments. It also adds a
`#mine` method that orchestrates the mining process.
@MatheusRich MatheusRich force-pushed the mr/gold-miner-rewriting branch from beba851 to a137d94 Compare October 23, 2023 15:50
@MatheusRich MatheusRich requested a review from elias19r October 27, 2023 14:48
Comment on lines +28 to +30
explorer: slack_explorer,
smith: GoldMiner::BlogPostSmith.new,
distributor: GoldMiner::TerminalDistributor.new
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the previous refactor PRs had this in mind. Now that we can swap parts of the process and produce new behaviors.

When we have a proper CLI, this will be handy.

@MatheusRich MatheusRich merged commit 9f6301d into main Oct 30, 2023
1 check passed
@MatheusRich MatheusRich deleted the mr/gold-miner-rewriting branch October 30, 2023 15:38
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.

1 participant