chore: correct license name - #624
Open
06kellyjac wants to merge 2 commits into
Open
06kellyjac wants to merge 2 commits into
06kellyjac wants to merge 2 commits into
Conversation
SPDX records this specific variant of the MIT license as "MIT +no-false-attribs license" or MITNFA for short.
Code Review by Qodo
1. compareVersions missing @example
|
Review Summary by QodoUpdate license to MITNFA (MIT +no-false-attribs)
WalkthroughsDescription• Update license identifier from MIT to MITNFA • Correct license name in source file headers • Regenerate documentation with updated license • Update package.json license field Diagramflowchart LR
A["License Files"] -- "Update to MITNFA" --> B["Source & Config"]
B -- "Regenerate" --> C["Documentation"]
C -- "Update metadata" --> D["package.json"]
File ChangesView more (9)6. docs/Bowser.html
|
Comment on lines
+551
to
+556
| /** | ||
| * Check if browser version equals the version or equals one of versions | ||
| * @param {(string|string[])} versionsOrVersion versions strings array or version string | ||
| * @returns {boolean} | ||
| */ | ||
| compareVersions(versionsOrVersion) { |
There was a problem hiding this comment.
1. compareversions missing @example 📘 Rule violation ⚙ Maintainability
The public Parser.compareVersions() method JSDoc lacks an @example tag, so the documentation is incomplete per the checklist. This reduces API clarity and makes the newly documented method harder to use correctly.
Agent Prompt
## Issue description
`Parser.compareVersions()` is a public method but its JSDoc does not include an `@example` tag, which is required by the compliance checklist.
## Issue Context
The method is documented with `@param` and `@returns`, but the missing `@example` prevents generated docs from showing a usage example.
## Fix Focus Areas
- src/parser.js[507-518]
- docs/parser.js.html[551-556]
- docs/Parser.html[321-477]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+321
to
+334
| <h4 class="name" id="compareVersions"><span class="type-signature"></span>compareVersions<span class="signature">(versionsOrVersion)</span><span class="type-signature"> → {boolean}</span></h4> | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| <dl class="details"> | ||
|
|
||
|
|
||
| <dt class="tag-source">Source:</dt> | ||
| <dd class="tag-source"><ul class="dummy"><li> | ||
| <a href="parser.js.html">parser.js</a>, <a href="parser.js.html#line512">line 512</a> | ||
| </li></ul></dd> |
There was a problem hiding this comment.
2. Missing ts compareversions type 🐞 Bug ≡ Correctness
The regenerated docs now document Parser.compareVersions(...), but index.d.ts does not declare this method on the Parser interface, so TypeScript users cannot call a documented runtime API without type errors.
Agent Prompt
## Issue description
The generated documentation now includes `Parser.compareVersions(versionsOrVersion)` as a public method, but the TypeScript declaration file `index.d.ts` does not declare `compareVersions` on the `Bowser.Parser.Parser` interface. This makes the documented API unusable for TypeScript consumers (compile-time error: property does not exist).
## Issue Context
- Runtime implementation exists in `src/parser.js`.
- Docs regeneration added `compareVersions` to `docs/Parser.html`, which effectively advertises it as supported API.
## Fix Focus Areas
- index.d.ts[49-110]
- Add a `compareVersions(versionsOrVersion: string | string[]): ...` method to the `Bowser.Parser.Parser` interface near `compareVersion`.
- Choose an accurate return type to match runtime behavior (recommended: `boolean | undefined`, unless you also change runtime to always return boolean).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The https://github.com/actions/dependency-review-action is identifying bowser as
MIT AND MITNFAlicensed:finos/git-proxy#1578 (comment)
https://github.com/finos/git-proxy/actions/runs/27081052911/job/79926837725
https://spdx.org/licenses/MITNFA.html
Looking at the additional 3rd paragraph of the license I think
MITNFAdoes indeed apply.I've taken the action to mark everything correctly as
MITNFAbut if the intention was to only haveMITother adjustments would need to be made.MIT OR MITNFAcould be another option but IMO would be a bit pointless. And in the case ofMIT AND MITNFA,MITNFAis justMITwith extra parts soMIT ANDis redundant.I used
npm run generate-docsto fix the content in thedocs/folder, some of the spacing seems wrong but I've kept it as the docs seem to be missing the newcompareVersionsfunction and the example.I'm happy to close this PR if you'd rather make the license adjustment yourselves. 🙂