-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
refactor: updated and decoupled contextService logic #72
refactor: updated and decoupled contextService logic #72
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good, but I have problem with IContextAllocator idea and implemenation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good, only this problem with console.warn
Please wait for Łukasz. We'll see what he says.
Please also change the title of PR to the chore:
or refactor:
because as I see, you didn't use new services in source code, but only define it, so it's not feat/fix :)
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
I see that @magicmatatjahu reviews, so no need to wait for me with approval as it is pure refactor. also, if it is pure refactor, nothing touching the functionality, this should not trigger a release but be a |
@Souvikns Please change title and merge commit with |
* feat: updated and decoupled contextService logic * feat: creating new context when root context file is missing. * feat: removed registry & stringify issue * chore: returning context or undefined in contextAllocator save method * feat: added warning for saving file. * feat: lint fixes
Description
This is the start of the process to move the context commands under config. In this PR I have focused on updating the
contextService
because of the issue discussed here. I would like to clarify what I have done here.validate
command as it needs only one spec file path. But to support libraries like cupid and diff where more than one spec file path is needed context shall evolve as well. For that, I have created a separateIContext
interface that could be easily updated.ContextAllocator
to load context files accordingly. For now, it is loading the context file from the root file itself. This also helps in testing allowing us to create mock classes to check functionalities.ContextService
does not throw an error if the contextFile is not present in the root dir instead it loads undefined context.Related issue(s)
See also #38 and #37