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

Added dictionary classes for badges removing them from services declaration #771

Merged
merged 1 commit into from
Apr 22, 2022
Merged

Added dictionary classes for badges removing them from services declaration #771

merged 1 commit into from
Apr 22, 2022

Conversation

AlessandroMinoccheri
Copy link
Member

In this PR I removed from the services file badges arrays and I added two classes to store information.

I would like to discuss:

  • the right position of those files
  • the correct name for those files, I used the same name used before but I would like to improve them

@AlessandroMinoccheri AlessandroMinoccheri force-pushed the create-dictionary-classes-for-badges branch from 83a4146 to 8bde988 Compare April 18, 2022 21:28
@AlessandroMinoccheri AlessandroMinoccheri linked an issue Apr 18, 2022 that may be closed by this pull request
Removing parameters from services adding dictionaries #769
Closed
garak
garak requested changes Apr 19, 2022
View reviewed changes
src/Service/Badges.php Outdated Show resolved Hide resolved
tests/Service/BadgesTest.php Outdated Show resolved Hide resolved
@AlessandroMinoccheri AlessandroMinoccheri force-pushed the create-dictionary-classes-for-badges branch 2 times, most recently from d3fe607 to 35b3cae Compare April 19, 2022 18:04
@AlessandroMinoccheri AlessandroMinoccheri changed the title WIP: added dictionary classes for badges removing them from services declaration Added dictionary classes for badges removing them from services declaration Apr 20, 2022
JellyBellyDev
JellyBellyDev requested changes Apr 22, 2022
View reviewed changes
Copy link
Member

@JellyBellyDev JellyBellyDev left a comment

Choose a reason for hiding this comment

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

GJ!!!! I only reported a couple of things! ;)

src/Service/AllInBadges.php Outdated Show resolved Hide resolved
tests/Service/SnippetGeneratorTest.php Outdated Show resolved Hide resolved
@AlessandroMinoccheri AlessandroMinoccheri force-pushed the create-dictionary-classes-for-badges branch from 35b3cae to 8fb0da7 Compare April 22, 2022 07:43
@AlessandroMinoccheri AlessandroMinoccheri force-pushed the create-dictionary-classes-for-badges branch from 8fb0da7 to ce376a2 Compare April 22, 2022 10:36
JellyBellyDev
JellyBellyDev approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@JellyBellyDev JellyBellyDev left a comment

Choose a reason for hiding this comment

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

LGTM

@JellyBellyDev JellyBellyDev merged commit 55f85e6 into master Apr 22, 2022
@JellyBellyDev JellyBellyDev deleted the create-dictionary-classes-for-badges branch April 22, 2022 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garak garak garak approved these changes

@JellyBellyDev JellyBellyDev JellyBellyDev approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Removing parameters from services adding dictionaries
3 participants
@AlessandroMinoccheri @garak @JellyBellyDev