-
Notifications
You must be signed in to change notification settings - Fork 293
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
feat(pack): add support for scaffolding new extensions #2041
base: main
Are you sure you want to change the base?
Conversation
update support for newExtension
add support for extension new
add support for scaffolding new extensions
enable NewExtension method in MockPackClient
enable support for new extension
enable newextension
add test for newExtension
add new Extension in client
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2041 +/- ##
==========================================
- Coverage 79.63% 79.30% -0.32%
==========================================
Files 176 177 +1
Lines 13214 13367 +153
==========================================
+ Hits 10522 10600 +78
- Misses 2022 2093 +71
- Partials 670 674 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
} | ||
|
||
func (e *ExtensionDescriptor) EnsureStackSupport(_ string, _ []string, _ bool) error { | ||
return nil | ||
} | ||
|
||
func (e *ExtensionDescriptor) EnsureTargetSupport(_, _, _, _ string) error { | ||
return nil | ||
func (e *ExtensionDescriptor) EnsureTargetSupport(os, arch, distroName, distroVersion string) error { |
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.
@jjbustamante are you aware of any existing logic we can re-use to validate that extensions support targets?
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.
I think this code is similar to this one maybe we can try to refactor it and externalize it to be reuse in both places?
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.
@jjbustamante could you please give some clarification on the appropriate directory for storing the refactored files!! Any additional information about the specific files or workflow would be super helpful!!
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.
@jjbustamante @natalieparellano I think a direct refactoring using a common function might not be feasible due to the structural differences (ExtensionDescriptor and BuildpackDescriptor). While the two structs share similarities, they do have some distinct fields.
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.
@sarthaksarthak9 thank you for this, the PR is progressing nicely. I left a few comments :)
thank you for reviewing :) |
remove the stack flag and update the commands
remove the unnecessary indentation
update the unit test
Add support for scaffolding new extensions
Output
Before
After
Documentation
Related
Resolves #1636