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

feat(packaging): add support for ipk package format #4863

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

Conversation

schmidtw
Copy link

Adds code to expose the ipk configuration values and registers the ipk package format with nfpm.

Updates the documentation with how to use the new ipk specific configuration parameters.

This isn't ready to merge, but I have some questions

  1. I copied the TestIPKSpecificConfig() (code) from the TestAPKSpecificConfig() and found it only is really testing if specific scripts are there or not. Since IPK doesn't have any additional scripts, just fields in one file do I need this test function? Is there a better way to validate the output?
  2. I have run into issues where the tests expect goreleaser to be the org & repo - is there a way to override this in my fork without changing the code?
  3. Is the ToNFPAlts() and ToNFP() living in the config.go file ok? There wasn't much code in that file, so I figured I'd ask if you'd rather this code be elsewhere.

@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 14, 2024
@schmidtw
Copy link
Author

@caarlos0 I'm not sure if GH will ping you on a draft PR so I'm mentioning you here to get some feedback when you get some time. Thank you!

Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

this looks good, thank you!

for full transparency: I plan to release v2 as 1.26 minus the deprecations, so this will probably land in v2.1 only.

I'm aiming to release v2 either this weekend or the next, depending if I get any bug reports during this time

Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.79%. Comparing base (c68d830) to head (a4320f7).
Report is 281 commits behind head on main.

Current head a4320f7 differs from pull request most recent head 41a5ef6

Please upload reports for the commit 41a5ef6 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4863      +/-   ##
==========================================
+ Coverage   83.77%   83.79%   +0.01%     
==========================================
  Files         135      139       +4     
  Lines       13029    11224    -1805     
==========================================
- Hits        10915     9405    -1510     
+ Misses       1677     1356     -321     
- Partials      437      463      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@schmidtw
Copy link
Author

Do you want me to update the doc to call out v2.1 in the PR & make this a non-draft PR?

@caarlos0
Copy link
Member

if you can yeah, I appreciate it! otherwise I can make the changes when the time comes :)

Adds code to expose the ipk configuration values and registers the ipk
package format with nfpm.

Updates the documentation with how to use the new ipk specific
configuration parameters.
@schmidtw schmidtw marked this pull request as ready for review May 17, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants