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

Restore semantic configuration to allow deep merge on import #898

Merged
merged 1 commit into from
Jun 4, 2022

Conversation

dbrumann
Copy link
Collaborator

@dbrumann dbrumann commented Jun 3, 2022

Restores semantic configuration for parameters:

  • paths
  • exclude_files
  • layers
  • ruleset
  • skip_violations
  • formatters
  • analyser
  • use_relative_path_from_depfile
  • ignore_uncovered_internal_classes

but not for:

  • projectDirectory
  • depfileDirectory
  • workingDirectory

Fixes #878

To Dos:

  • Update documentation
  • Update examples
  • Check if imports works as expected again
  • Check backwards compatibility

@patrickkusebauch
Copy link
Collaborator

Tried this branch on my repo and it had no effect. :(

@dbrumann
Copy link
Collaborator Author

dbrumann commented Jun 4, 2022

@patrickkusebauch There was a problem where services were loaded after the extension. It should be better now.

I created a new example for imports docs/examples/imported_layers_deptrac.yaml based on docs/examples/DirectoryLayer.depfile.yaml. It's the same configuration but somehow still behaves differently. I am investigating what's causing this.

@dbrumann dbrumann force-pushed the deptrac_config branch 5 times, most recently from 03d69de to ce1c092 Compare June 4, 2022 09:08
@dbrumann
Copy link
Collaborator Author

dbrumann commented Jun 4, 2022

Turns out, I am just bad at copy & paste and Layer2 accidentally contained Layer1 classes. 😄

I still need to double check if I missed any parameters, but this should be good to go.

@dbrumann dbrumann marked this pull request as ready for review June 4, 2022 09:15
@patrickkusebauch
Copy link
Collaborator

I can test it later today if you'd like

@dbrumann dbrumann force-pushed the deptrac_config branch 3 times, most recently from e4c65d6 to a255cfb Compare June 4, 2022 09:40
@patrickkusebauch
Copy link
Collaborator

Listing issues as I encounter them:

Unrecognized option "count_use_statements" under "deptrac.analyser". Available option is "types".

@patrickkusebauch
Copy link
Collaborator

Unrecognized option "baseline" under "deptrac". Available options are "analyser", "exclude_files", "formatters", "ignore_uncovered_internal_classes", "layers", "paths", "ruleset", "skip_violations", "use_relative_path_from_depfile".

@patrickkusebauch
Copy link
Collaborator

Otherwise it is working.

@dbrumann
Copy link
Collaborator Author

dbrumann commented Jun 4, 2022

Unrecognized option "count_use_statements" under "deptrac.analyser". Available option is "types".

I purposely didn't include it because it was deprecated anyway. If I remember correctly this option should be equivalent to this:

deptrac:
  analyzer:
    types: ['class']

Unrecognized option "baseline" under "deptrac".

This is equivalent to imports:, I'd prefer not to add baseline and use that instead.

Cannot autowire service "1": argument "$layers" of method "DanceEngineer\DeptracAwesome\UnusedDependenciesCommand::__construct()" is type-hinted "array", you should configure its value explicitly.

I don't see what's causing this. Can you give more details for this problem?

@patrickkusebauch
Copy link
Collaborator

Ad 1: Ok, that's fine. I had it there anyways just for legacy reasons
Ad 2: Sounds like a good idea. But it needs the corresponding docs.
Ad 3: That was my fault, it is a non-issue. I copied something incorrectly on my side.

@dbrumann dbrumann merged commit 4f4d499 into qossmic:main Jun 4, 2022
@dbrumann dbrumann deleted the deptrac_config branch June 4, 2022 15:33
Copy link
Collaborator

@patrickkusebauch patrickkusebauch left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@WayneStilwell
Copy link

@dbrumann I'm super excited about this! Do you have an estimate of when version 0.21 will be released?

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.

imports: does not work
3 participants