[format] Use thousands separator to improve readability

[strongly-typed]

After looking too long for a 10x error in frequency, I discovered this nice new feature.

How to deal with the inconsistency in the comment of vl53l0.hpp?

[strongly-typed]

Any more changes suggested?

[salkinium]

No, but the CI is broken. It appears to be running on your CircleCI instance, which isn‘t useful since yours doesn‘t have all the secret deploy keys etc.

[salkinium]

Ping?

[strongly-typed]

Ping?

Yes, what can I do?

[salkinium]

The CI is broken, because it runs on your CircleCI instance. Not sure why.

[strongly-typed]

The CI is broken, because it runs on your CircleCI instance. Not sure why.

I think this is the default as it is still my branch and not (yet) part of github.com/modm-io/modm.

You could pull the branch from me and then push it to modm-io. Then modm-io's CircleCI instance will build it.

Alternatively, you probably could have a read here.

[strongly-typed]

I meanwhile rebased to current develop

[strongly-typed]

So what is happening with CircleCI?

I have an account with CircleCI, so my branches from Github are built with CircleCI, even without me having opened a PR here at Github. These jobs are named after my Github branches:

In a PR, Github is reusing these jobs to avoid rerunning the same jobs again.

I assume: For PRs from contributors without a CircleCI account, the jobs are created in the modm-io account of CircleCI, like with #228

https://circleci.com/gh/modm-io/modm/11870

As modm's CircleCI config instructs CircleCI to run the build-docs job only if the job name matches pull it is not run for my feature branch in my CircleCI instance, which are named after my branches.

@salkinium How to deal with it? What was the motivation to build docs only for branches with name beginning with pull/?

[strongly-typed]

Yeah!

[salkinium]

There are two docs related jobs, the build-docs builds the docs without depending on the long running compile-all jobs (f4 and f7) so it is up to 3mins faster, due to improved parallel utilization.
The build-docs-upload job depends on all compile-all jobs and uploads the docs as well.

I utterly disagree with github/circleci that they should be reusing build jobs from another account, that makes absolutely no sense and is a complete liability. I’ve never heard of a CI where the user provides the passing checks. I cannot check the environment of your instance, neither should I need to check your instance for conformity. And I am most certainly not sharing the commit key to modm.io.

There must be a setting for this, I just haven‘t found it yet.

[salkinium]

No.

[strongly-typed]

There must be a setting for this, I just haven‘t found it yet.

Might this enable building the PRs inside of the modm-io organisation on CircleCI?
What is the setting current of this for modm-io?

Still a very interesting config. What if I PR'ing something with echo $CI_SECRET? What is the environment in my PR is then build? Is it the one of modm-io or mine?

Questions, questions, questions ...

[salkinium]

What is the setting current of this for modm-io?

It's set to On otherwise it would build no pull requests at all (yes I've tested this).

What if I PR'ing something with echo $CI_SECRET?

The access to modm.io is done via SSH key. From the docs:

The private keys of the checkout keypairs we generate never leave our systems (only the public key is transmitted toGitHub), and are safely encrypted in storage. However, since they are installed into your build containers, any code that you run in CircleCI can read them. You shouldn't push untrusted code to CircleCI!

Lolwut? I guess anyone can just echo out the private key?

add_ssh_keys
cat ~/.ssh/*

Why do I even bother with this shit?

[salkinium]

I'm just going to ignore this CI issue for now… *pouts*

[salkinium]

Thanks

[strongly-typed]

I'm just going to ignore this CI issue for now… pouts

A better condition for build-docs might include something from the CircleCI environment. Let me try this before merging.

[strongly-typed]

@salkinium Would you accept the reformulated condition for the pull request in CircleCI?

[salkinium]

I think the canonical fix would be to change the branch regex to include feature/ and fix/ name prefixes, or to use the negated version, ie. run iff not develop (which I couldn‘t get to work, probably too dumb for it).

[salkinium]

Specifically, I could not get the ignore key working.

[strongly-typed]

I think the canonical change would be to change the branch regex to include feature/ and fix/ name prefixes, or to use the negatesd version, ie. run iff not develop (which I couldn‘t get to work, probably too dumb for it).

Could you describe when the build-doc job should run? For every commit while developing a feature (on a feature or fix branch) or only when the PR is opened? I did not yet get your use case

[salkinium]

The build-doc could be run in parallel to the compile all jobs, except for develop where it must run last in order to NOT push to modm.io in case something does fail in compile all. It‘s a last defense in that sense. I like the way it is now, since the build docs jobs is visually separated from the compile all jobs but still runs in parallel.

It‘s a matter of defining the right dependencies, which can only be done with two separate jobs.

[strongly-typed]

Btw, is the if [ "$CIRCLE_BRANCH" = "develop" ]; then redundant with the only: develop option?

build-docs-upload and build-docs share some code.

Shall we have the build-docs-upload "just" do the upload and have it depended on everything plus build-docs and have the only: develop option?

Then no condition for build-docs is required and it can always run.

[salkinium]

Yes the branch check is redundant. You cannot share state or data across jobs, due to the container thing. So you still have to build even if you only upload.

[strongly-typed]

There are artifacts and workspaces.

Let me take a look soon.

[salkinium]

I'm going to merge the PR without your changes to the CI, since you can work on that independently of the (code) changes in this PR.

[salkinium]

Merged manually.