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

Fix duplicate --env options #278

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix duplicate --env options #278

wants to merge 2 commits into from

Conversation

cllns
Copy link
Member

@cllns cllns commented Nov 26, 2024

With Hanami 2.2.1, --env shows up twice for generate commands.

Options:
  --env=VALUE, -e VALUE             # App environment (development, test, production)
  --slice=VALUE                     # Slice name
  --env=VALUE, -e VALUE             # App environment (development, test, production)
  --help, -h                        # Print this help

This is due to klass.prepend(Environment) being run twice whenever we subclass App::Command (like we do for App::Generate::Command)

This change fixes that... but it breaks our specs for the feature that automatically runs db commands in test env when running them for development, from #247

I think it's because the prepending the Environment module several times ensures it's at the top of the ancestors list.

For hanami db migrate failure:

# self.class.ancestors inside the Migrate command manually formatted for reading
[
  Hanami::CLI::Commands::App::Command::Environment
  Hanami::CLI::Commands::App::DB::Migrate, 
  Hanami::CLI::Commands::App::Command::Environment, 
  Hanami::CLI::Commands::App::DB::Command, 
  Hanami::CLI::Commands::App::Command, 
  Hanami::CLI::Command, 
  Dry::CLI::Command, 
  Object, 
  JSON::Ext::Generator::GeneratorMethods::Object, 
  Kernel, 
  BasicObject
]

You can see Environment is duplicated there, and at the start of the list.

UPDATE I solved this at the point of adding the option rather than including the module and CI passes now, and only one --env option shows up, so the double prepare doesn't appear to cause any problems.

Since we subclass this App::Command class as App::Generate::Command,
the Environment module was being included twice
@cllns cllns marked this pull request as draft November 26, 2024 03:33
@cllns cllns changed the title [WIP] Only include one --env option Fix duplicate --env options Nov 27, 2024
@cllns cllns marked this pull request as ready for review November 27, 2024 02:45
@cllns cllns requested a review from timriley November 27, 2024 02:45
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