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

events: Added note about immediate firing of periodic events #6274

Merged
merged 1 commit into from Mar 15, 2018

Conversation

Projects
None yet
8 participants
@geky
Member

geky commented Mar 5, 2018

Description

Added note about immediate firing of periodic events

see #6256
cc @pauluap, @kjbracey-arm

Pull request type

  • Fix
  • Refactor
  • New target
  • Feature
  • Breaking change
@pauluap

This comment has been minimized.

pauluap commented Mar 5, 2018

The Event class has no documentation :) But it's an very good note I think, alerts the reader that something's up and if they're sufficiently motivated, they can check on it themselves.

@geky

This comment has been minimized.

Member

geky commented Mar 5, 2018

Oh wow, there really isn't...

There is doxygen in the codebase, but for some reason it isn't generated. Will have to look into this:
https://github.com/ARMmbed/mbed-os/blob/master/events/Event.h#L1707

@AnotherButler, just fyi

@geky

This comment has been minimized.

Member

geky commented Mar 5, 2018

Also @AnotherButler quick question, when is doxygen updated on the developer site? Is it every minor release?

@pauluap

This comment has been minimized.

pauluap commented Mar 5, 2018

@geky

This comment has been minimized.

Member

geky commented Mar 5, 2018

That's strange, it looks like all the comments were stripped out of the file? The Event.h file hasn't been touched in a long time (9 months)...

@cmonr cmonr requested a review from kjbracey-arm Mar 6, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

@adbridge Thoughts to above?

@cmonr

cmonr approved these changes Mar 6, 2018

@adbridge

This comment has been minimized.

Contributor

adbridge commented Mar 6, 2018

@geky @cmonr If there used to be doxygen style comments in the event.h header and they have been stripped out , we should track down when and why this was done and look to replace it.....

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

/morph build

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

The last PR that touched the header file: #4434

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

On second glance, what's currently in the mbed repo looks fine. Wondering if the doc generation isn't capturing this/these files.

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 6, 2018

Build : SUCCESS

Build number : 1363
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6274/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

mbed-ci commented Mar 6, 2018

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

Not restarting CI quite yet.

@studavekar What do you take of the export failure?

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Mar 6, 2018

Not restarting CI quite yet.

@studavekar What do you take of the export failure?

I am not sure, at least the failure look legit but in no way related to this change.

  158 000000a8                 END
0 Errors, 1 Warning
Error: L6200E: Symbol __asm___14_system_clock_c_a9c8fbbb____REV16 multiply defined (by mbed-os/targets/TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_rcc_ex.o and mbed-os/targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F091RC/device/system_clock.o).
Error: L6200E: Symbol __asm___14_system_clock_c_a9c8fbbb____REVSH multiply defined (by mbed-os/targets/TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_rcc_ex.o and mbed-os/targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F091RC/device/system_clock.o).
Finished: 0 information, 0 warning and 2 error messages.
make[1]: *** [mbed-os-example-blinky.elf] Error 1
@cmonr

This comment has been minimized.

Contributor

cmonr commented Mar 6, 2018

Alright, gonna see if this goes 2/2.
/morph export-build

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit b97c7d8 into ARMmbed:master Mar 15, 2018

19 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Local events testing has passed
Details
travis-ci/littlefs Passed, code size is 10060B (+0.00%)
Details
travis-ci/mbed2-ATMEL Local mbed2-ATMEL testing has passed
Details
travis-ci/mbed2-MAXIM Local mbed2-MAXIM testing has passed
Details
travis-ci/mbed2-NORDIC Local mbed2-NORDIC testing has passed
Details
travis-ci/mbed2-NUVOTON Local mbed2-NUVOTON testing has passed
Details
travis-ci/mbed2-NXP Local mbed2-NXP testing has passed
Details
travis-ci/mbed2-RENESAS Local mbed2-RENESAS testing has passed
Details
travis-ci/mbed2-SILICON_LABS Local mbed2-SILICON_LABS testing has passed
Details
travis-ci/mbed2-STM Local mbed2-STM testing has passed
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Mar 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment