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.

Sign up for GitHub

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

Jump to bottom

refactor: Separate main function from MainCommand #1724

Merged
merged 1 commit into from
Mar 23, 2021
Merged

refactor: Separate main function from MainCommand #1724

merged 1 commit into from
Mar 23, 2021

Conversation

jan-goral
Copy link
Contributor

Fixes #1723

Test Plan

It's just refactoring, all unit tests should pass.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2021 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2021 •
edited
Loading

Timestamp: 2021-03-23 12:19:44
Buildscan url for ubuntu-workflow run 679506955

Sloox
Sloox previously requested changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@Sloox Sloox left a comment

Choose a reason for hiding this comment

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

Does it require any updates for tests? or atleast a documentaiton of what the change introduces (If anybesides neater code)

@bootstraponline bootstraponline force-pushed the 1723-Separate_main_function_from_MainCommand branch from f7b2586 to 2217e2b Compare March 23, 2021 08:32
@bootstraponline bootstraponline force-pushed the 1723-Separate_main_function_from_MainCommand branch from 2217e2b to b43580c Compare March 23, 2021 12:13
@jan-goral
Copy link
Contributor Author

jan-goral commented Mar 23, 2021 •
edited
Loading

Does it require any updates for tests? or atleast a documentaiton of what the change introduces (If anybesides neater code)

This is refactoring, so it doesn't bring any changes in logic, so it also doesn't require any new test. But it brings some updates in tests according to separating the main function from MainCommand.

The reason for this change is specified here:

The documentation already takes this change into account, it is specified on the diagram. The main function and MainCommand are separated:

@pawelpasterz pawelpasterz enabled auto-merge (squash) March 23, 2021 13:59
@jan-goral jan-goral requested a review from Sloox March 23, 2021 14:01
@jan-goral jan-goral dismissed Sloox’s stale review March 23, 2021 14:22

Change request must be a statement not a question.

@pawelpasterz pawelpasterz merged commit fdce797 into master Mar 23, 2021
@pawelpasterz pawelpasterz deleted the 1723-Separate_main_function_from_MainCommand branch March 23, 2021 15:23
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@piotradamczyk5 piotradamczyk5 piotradamczyk5 approved these changes

@pawelpasterz pawelpasterz pawelpasterz approved these changes

@adamfilipow92 adamfilipow92 Awaiting requested review from adamfilipow92

@zuziaka zuziaka Awaiting requested review from zuziaka

@Sloox Sloox Awaiting requested review from Sloox

Assignees

@jan-goral jan-goral

Labels
Refactor Snake 🐍
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Separate main function from MainCommand
4 participants
@jan-goral @Sloox @pawelpasterz @piotradamczyk5