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/umorse: Add kconfig option #15980

Closed
wants to merge 2 commits into from

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

Add the umorse pkg to the pkg rsource and expose dependencies.

Testing procedure

TEST_KCONFIG=1 make menuconfig -C tests/pkg_umorse/

Issues/PRs references

Split from #15950

@MrKevinWeiss MrKevinWeiss added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer 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 Area: Kconfig Area: Kconfig integration labels Feb 10, 2021
@MrKevinWeiss MrKevinWeiss self-assigned this Feb 10, 2021
@leandrolanzieri leandrolanzieri removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 11, 2021
Copy link
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

You should include the pkg_umorse test application to the ones that murdock tests with Kconfig

RIOT/.murdock

Lines 27 to 28 in d7f3e92

: ${TEST_KCONFIG_native:="examples/hello-world tests/periph_* tests/sys_crypto
tests/prng_* tests/xtimer_* tests/ztimer_* tests/driver_ws281x"}

With the corresponding app.config.test.

#
config USEPKG_UMORSE
bool "umorse"
select USEMODULE_POSIX_SLEEP
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
select USEMODULE_POSIX_SLEEP
select MODULE_POSIX_SLEEP

Copy link
Contributor

Choose a reason for hiding this comment

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

We should check if it is safe to keep the select or this should be depends on. The posix modules are not modelled AFAIK, so those should be added.

# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.
#
config USEPKG_UMORSE
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
config USEPKG_UMORSE
config PACKAGE_UMORSE

config USEPKG_UMORSE
bool "umorse"
select USEMODULE_POSIX_SLEEP

Copy link
Contributor

Choose a reason for hiding this comment

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

Check this new blank line error please.

Add the umorse pkg to the pkg rsource and expose dependancies.
@MrKevinWeiss
Copy link
Contributor Author

Done and squashed!

@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 11, 2021
@MrKevinWeiss
Copy link
Contributor Author

Don't know why I have 2 of these #15985

@miri64
Copy link
Member

miri64 commented Feb 19, 2021

I was confused when I saw this in my MUA and only then I understood.

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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants