Skip to content

Add better support for SPDX headers#1268

Merged
pfitzseb merged 1 commit intojulia-vscode:masterfrom
Seelengrab:spdx_licenses
Feb 22, 2024
Merged

Add better support for SPDX headers#1268
pfitzseb merged 1 commit intojulia-vscode:masterfrom
Seelengrab:spdx_licenses

Conversation

@Seelengrab
Copy link
Copy Markdown
Contributor

@Seelengrab Seelengrab commented Feb 19, 2024

This commit does these things:

  • Add rudimentary detection of EUPL-* licenses
  • Add logging statements throughout the action to make it debuggable should some license not be properly detected
  • Fix a bug with SPDX header detection in existing files

To expand on the last point - previously, the header detection didn't have multiline matching enabled on its regex, which meant that any file having any lines other than the SPDX header wasn't detected as actually having a header. That's a bit bad, since presumably there would be actual source code following that header.

For a visual example, before:

image

After:

image

As for tests - I looked at test/requests/test_actions.jl and found an extremely minimal test for that action. I have no idea how to add a test for the functionality there, so I'd appreciate any guidance you can offer.

For every PR, please check the following:

@Seelengrab
Copy link
Copy Markdown
Contributor Author

Seelengrab commented Feb 19, 2024

Since this bumps compat to 1.3 for the artifact, the failure on <1.3 is entirely expected - I hope this can still go through in some form, either actually dropping support for <1.3 or in some manner that allows better detection of licenses other than MIT.

@pfitzseb
Copy link
Copy Markdown
Member

Unfortunately we're fairly committed to keeping 1.0 compat for now.

So, while being able to identify the license is probably useful, I think just fixing the regex is probably best for now. Also cc @fredrikekre because I think he originally implemented this?

@Seelengrab
Copy link
Copy Markdown
Contributor Author

Seelengrab commented Feb 19, 2024

That's unfortunate to hear, especially since 1.0 has been out of language support for 2+ years now. I'd have loved to do some PRs with other code actions, but I don't really want to go back to the time of slow compilation :) Maybe this can be done with the next LTS?

I'll adjust the PR so only a very minimal version is done. Specifically:

  • Fixing the regex, so the SPDX header is identified correctly
  • Logging
  • Adding detection of EUPL licenses the way it's done for MIT

This commit does these things:

 * Add rudimentary detection of EUPL-* licenses
 * Add logging statements throughout the action to make it debuggable
   should some license not be properly detected
 * Fix a bug with SPDX header detection in existing files

To expand on the last point - previously, the header detection
didn't have multiline matching enabled on its regex, which meant
that any file having any lines other than the SPDX header wasn't
detected as actually having a header. That's a bit bad, since presumably
there would be actual source code following that header.
@Seelengrab
Copy link
Copy Markdown
Contributor Author

I've removed the artifact & compat bumpage, does this need anything else?

@pfitzseb pfitzseb merged commit 353a345 into julia-vscode:master Feb 22, 2024
@pfitzseb
Copy link
Copy Markdown
Member

Nope! Thanks again for the fix.

@Seelengrab
Copy link
Copy Markdown
Contributor Author

Thank you!

@fredrikekre
Copy link
Copy Markdown
Member

I'd have loved to do some PRs with other code actions

I think it is fine to have some actions behind a version check so don't let the Julia 1.0 support discourage you.

@pfitzseb
Copy link
Copy Markdown
Member

Yes, but that's a bit tricky when it comes to dependencies.

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.

3 participants