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

Configuration console command #54

Merged
merged 7 commits into from
Nov 20, 2023
Merged

Conversation

misantron
Copy link
Contributor

@misantron misantron commented Oct 25, 2023

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Configuration console command to display config settings

Issue ticket number and link

#48

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added/updated tests.

@shcherbanich shcherbanich requested a review from seanmcn October 25, 2023 10:42
@misantron
Copy link
Contributor Author

@shcherbanich should configuration command support all configuration keys or just collections?

@shcherbanich
Copy link
Contributor

Ideally, this command should support the output of all keys

@misantron misantron marked this pull request as ready for review October 29, 2023 20:25
Copy link
Collaborator

@seanmcn seanmcn left a comment

Choose a reason for hiding this comment

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

Overall it looks good thank you!

I've requested some minor changes around logging messages.

I also think it would be nice if the command could output all configuration, as a user will be unlikely to iniitally know the config keys available.

Finally, there are merge conflicts on the branch (due to the generated documentation), so please merge 1.x.x into the branch again & regenerate the documentation.

Thanks 😃

src/Console/Command/ConfigurationCommand.php Outdated Show resolved Hide resolved
src/Core/Configuration/Configuration.php Show resolved Hide resolved
src/Core/Configuration/Configuration.php Show resolved Hide resolved
src/Core/Configuration/Configuration.php Show resolved Hide resolved
@misantron misantron requested a review from seanmcn November 14, 2023 14:39
# Conflicts:
#	docs/shared_c.cache
#	docs/tech/map.md
#	src/Console/App.php
@seanmcn seanmcn merged commit 283955f into bumble-tech:1.x.x Nov 20, 2023
1 check passed
@seanmcn
Copy link
Collaborator

seanmcn commented Nov 20, 2023

@misantron thank you for your contribution!

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.

3 participants