Skip to content

Conversation

@RSingh1511
Copy link
Contributor

Notes for Reviewer

Pre-Review Checklist for the PR Author

  • PR title is short, expressive and meaningful
  • Commits are properly organized
  • Relevant issues are linked in the References section
  • Tests are conducted
  • Unit tests are added

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

Post-review Checklist for the PR Author

  • All open points are addressed and tracked via issues

References

Closes #

@github-actions
Copy link

github-actions bot commented Feb 11, 2026

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
2026/02/12 12:45:35 Downloading https://releases.bazel.build/8.3.0/release/bazel-8.3.0-linux-x86_64...
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 36184e86-b6b6-4b03-a418-df38e5ccd946
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'rules_python', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'bazel_skylib', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_rust', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_cc', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'aspect_rules_lint', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'buildifier_prebuilt', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'score_docs_as_code', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'googletest', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 4 packages loaded
Loading: 4 packages loaded
    currently loading: 
Loading: 4 packages loaded
    currently loading: 
WARNING: Target pattern parsing failed.
ERROR: Skipping '//:license-check': no such target '//:license-check': target 'license-check' not declared in package '' defined by /home/runner/work/logging/logging/BUILD
ERROR: no such target '//:license-check': target 'license-check' not declared in package '' defined by /home/runner/work/logging/logging/BUILD
INFO: Elapsed time: 13.310s
INFO: 0 processes.
ERROR: Build did NOT complete successfully
ERROR: Build failed. Not running target

@github-actions
Copy link

The created documentation from the pull request is available at: docu-html

@RSingh1511 RSingh1511 marked this pull request as ready for review February 11, 2026 09:33
@PiotrKorkus
Copy link
Contributor

Hi @RSingh1511,

I originally mentioned the files with BMW header:
ERROR: Missing copyright header in: score/datarouter/include/dlt/dlt_protocol.h
ERROR: Missing copyright header in: score/datarouter/include/dlt/dlt_types.h
ERROR: Missing copyright header in: score/datarouter/include/dlt/dlt_common.h
ERROR: Missing copyright header in: score/datarouter/include/daemon/i_diagnostic_job_parser.h

Those checks were disabled for now until Anton is back as he is the only codeowner that can approve those changes.
https://github.com/RSingh1511/logging/blob/eee4c40189904bfc2b9e42ee14d5672021a9bbbd/BUILD#L27

Could you fix those and re-enable cr check for /score?

@MaximilianSoerenPollak
Copy link

Normally with tooling 1.1.0 2026 should not be a problem in the copyright header anymore.

- Add Eclipse Foundation headers to dlt_protocol.h, dlt_types.h, dlt_common.h
- Fix duplicate header in i_diagnostic_job_parser.h
- Re-enable copyright check for score
Copy link
Contributor

@arkjedrz arkjedrz left a comment

Choose a reason for hiding this comment

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

Rust part is from 2026, issue must be elsewhere.

Comment on lines 1 to 29
Copy link
Contributor

Choose a reason for hiding this comment

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

is it correct to have both headers same time?

Choose a reason for hiding this comment

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

As far as I know it theoretically is acceptable.
I do not remember what the outcome of the discussion was on how to handle this (e.g. Update the Year or do this).

Though my gut tells me just updating the year is correct (if the file has changed)

@RSingh1511
Copy link
Contributor Author

Rust part is from 2026, issue must be elsewhere.

Copyright year should match when the file was modified (2025), not the current date (2026). This was causing the copyright checker to fail.

Copy link
Contributor

@arkjedrz arkjedrz left a comment

Choose a reason for hiding this comment

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

Added explicit notes - files first implemented in 2026 couldn't be last modified in 2025.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@pawelrutkaq pawelrutkaq left a comment

Choose a reason for hiding this comment

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

Fix other comments please

@RSingh1511
Copy link
Contributor Author

Added explicit notes - files first implemented in 2026 couldn't be last modified in 2025.

Restore 2026 copyright for files implemented in January 2026

* @licence app begin@
* SPDX license identifier: MPL-2.0
*
* Copyright (C) 2011-2015, BMW AG
Copy link
Contributor

Choose a reason for hiding this comment

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

I did fast check and:

3. Can you relicense MPL-2.0 code as Apache-2.0?
No (in general).

Imho this shall disappear (if you have a rights to do so) or we need eclipse guys or remove code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this!

@antonkri @rmaddikery - Please advise on handling these MPL-2.0 files

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also mention that this is Already a problem as this code is in repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, the MPL-2.0 files were already there. I'll revert my changes to them and keep the copyright check disabled for score/ as it was before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey everyone,

my understanding (may be a wrong one) was, that MPL-2.0 license for third code is allowed: https://www.eclipse.org/legal/licenses/ .

So, we are basically reusing third party code from dlt and this one is under MPL license, which is in the approved list of third party licesense.

Do I misunderstand smth.?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ye, I am just saying that probably you cannot have one file under MPL and Apache. But I dont mind if someone knows more, once You fix it feel free to approve ;) I jsut dont know probably ;) @RSingh1511 the check MUST be enabled ASAP - we cannot leave it disabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pawelrutkaq now I get it, thanks!

@RSingh1511 please remove apache license header and keep MPL license for the files, where the code was copied from DLT project. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antonkri @pawelrutkaq - I can't satisfy both requirements:

  • MPL-only headers → copyright check fails
  • Enable copyright check → needs Apache headers on all

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @RSingh1511 ,

as I understand, the cr_checker tools allows to specify exclusions, see eclipse-score/tooling#101

Could you please adapt the workflow to exclude those two dlt files with MPL license from the check?

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @antonkri ,

Implemented! Added copyright_exclusions file. Copyright check now passes with MPL-2.0 files excluded.

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants