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

Add missing license files & do minor updates to dot files #7161

Merged
merged 9 commits into from
Mar 21, 2025

Conversation

mhucka
Copy link
Contributor

@mhucka mhucka commented Mar 19, 2025

After discovering this week that Google's requirements for license headers are not what I originally thought (tl;dr: they're actually needed everywhere), I'm going around and adding them to files where I failed to put them before. Poor use of time, but seems necessary.

While at it, for files like .editorconfig, I'm updating them slightly to match the conclusions from recent discussions.

Apparently I misread guidance somewhere, and license headers are
required for these sorts of files after all.
Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.13%. Comparing base (3ae4ddb) to head (782c814).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7161      +/-   ##
==========================================
- Coverage   98.13%   98.13%   -0.01%     
==========================================
  Files        1093     1093              
  Lines       95579    95579              
==========================================
- Hits        93798    93797       -1     
- Misses       1781     1782       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mhucka mhucka marked this pull request as ready for review March 19, 2025 14:58
@mhucka mhucka requested review from vtomole and a team as code owners March 19, 2025 14:58
Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

Please see inline comments.

@mhucka
Copy link
Contributor Author

mhucka commented Mar 19, 2025

@Pavol the latest update address all the points, but does not add max_line_length yet, pending a resolution to our off-line discussion about what to do with that setting.

@mhucka mhucka self-assigned this Mar 19, 2025
mhucka and others added 3 commits March 19, 2025 16:36
After more discussion, the conclusion is that despite the issues with
`max_line_length`, it is handled as expected in the editors that many
people have been using.
Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@pavoljuhas
Copy link
Collaborator

pavoljuhas commented Mar 20, 2025

This patch resolves typescript lint check. The typescript linter looks at max_line_length and if active wants to reformat sources. That can be done later.

diff --git a/.editorconfig b/.editorconfig
index 97b035e1..b45c549c 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -38,4 +38,5 @@ max_line_length = 100
 # Google style guidelines use 2.
 indent_size = 2
+max_line_length = 0
 
 [*.json]

@mhucka
Copy link
Contributor Author

mhucka commented Mar 20, 2025

This patch resolves typescript lint check. The typescript linter looks at max_line_length and if active wants to reformat sources. That can be done later.

I'm curious: how did you figure this out?

mhucka added 2 commits March 20, 2025 07:43
The typescript linter looks at max_line_length and takes it into
account if it finds a value. This is causing CI checks to fail. We
need to reformat the sources in question and then we can remove this
setting again
@pavoljuhas
Copy link
Collaborator

pavoljuhas commented Mar 20, 2025

I'm curious: how did you figure this out?

I ran check/ts-lint locally. It was passing for the main and failing for the PR.
The linter messages asked for joining and reformatting of source lines; the only related setting in the PR was for the line length.

@mhucka mhucka added this pull request to the merge queue Mar 21, 2025
Merged via the queue into quantumlib:main with commit 4bdf21f Mar 21, 2025
38 checks passed
@mhucka mhucka deleted the mh-add-license-headers branch March 21, 2025 03:03
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.

2 participants