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

pkg/lvgl: add extra widget dependency #17760

Merged
merged 1 commit into from
Mar 28, 2022

Conversation

fjmolinas
Copy link
Contributor

Contribution description

Just a missing dependency, not sure it's woth the full build.

Testing procedure

Add the module, see missing files.

@fjmolinas fjmolinas requested a review from aabadie March 7, 2022 08:39
@github-actions github-actions bot added the Area: pkg Area: External package ports label Mar 7, 2022
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 7, 2022
@fjmolinas fjmolinas changed the title pkg/lvgl: add extra diget dependency pkg/lvgl: add extra widiget dependency Mar 8, 2022
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

@benpicco, any reason why you approved this PR without comment ?

I checked in LVGL code and there's indeed an implicit dependency between tabview and btnmatrix widgets but it's not specified explicitly in their code, neither in the tabview.c nor in their Kconfig.
That should be fixed upstream.

Since we are doing the same automatic dependency resolution with the lv_win extra widget, there's no reason to also do it for tabview.

I'm blocking because you forgot to update Kconfig.

@aabadie aabadie changed the title pkg/lvgl: add extra widiget dependency pkg/lvgl: add extra widget dependency Mar 8, 2022
@benpicco
Copy link
Contributor

benpicco commented Mar 8, 2022

It appeared to be a simple enough change - but yea, if the pkg provides Kconfig that needs to be updated as well

@fjmolinas
Copy link
Contributor Author

I have some more down the pipe, I'll bundle them together before fixing Kconfig

@fjmolinas
Copy link
Contributor Author

@aabadie I added Kconfig

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK

@aabadie aabadie enabled auto-merge March 28, 2022 08:34
@aabadie aabadie merged commit ef4b453 into RIOT-OS:master Mar 28, 2022
@fjmolinas
Copy link
Contributor Author

Thanks @aabadie and @benpicco!

@fjmolinas fjmolinas deleted the pkg_lvgl_widget_dep branch March 28, 2022 08:45
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants