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 warning diagnostic when using V2 api #33

Closed
wants to merge 7 commits into from

Conversation

microbit-robert
Copy link

@microbit-robert microbit-robert commented May 3, 2024

  • Translation sync required before merge and use (or at least addition of new strings in all simplified language files in English for now). Keeping in draft until this is done.

Comment on lines +391 to +401
maybeAddMicrobitVersionWarningBinderWrapper(importResult.importName, () =>
this._addDiagnostic(
this._fileInfo.diagnosticRuleSet.reportMicrobitV2ApiUse,
DiagnosticRule.reportMicrobitV2ApiUse,
Localizer.Diagnostic.microbitV2ModuleUse().format({
moduleName: importResult.importName,
device,
}),
node
)
);
Copy link
Author

@microbit-robert microbit-robert May 7, 2024

Choose a reason for hiding this comment

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

This is setup with a callback so we can track micro:bit specific diagnostics in one file, microbitUtils.ts. Useful if we only want to warn on the first use of a V2 API call in the future. We can't directly pass in this_.addDiagnostic as this causes errors.

Comment on lines 39 to 45
return (
['log', 'microbit.microphone', 'microbit.speaker', 'power'].includes(moduleName) ||
(moduleName === 'microbit' &&
['run_every', 'set_volume', 'Sound', 'pin_logo', 'pin_speaker'].includes(name ?? '')) ||
(moduleName === 'microbit.audio' && name === 'SoundEffect') ||
(moduleName === 'neopixel' && ['fill', 'write'].includes(name ?? ''))
);
Copy link
Author

Choose a reason for hiding this comment

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

Hardcoded, but only in one place. Are any missing?

Comment on lines +61 to +63
"microbitV2ClassMethodUse": "The \"{methodName}\" method on the \"{className}\" class cannot be used with a {device}",
"microbitV2ModuleMemberUse": "\"{name}\" from the \"{moduleName}\" module cannot be used with a {device}",
"microbitV2ModuleUse": "\"{moduleName}\" cannot be used with a {device}",
Copy link
Author

Choose a reason for hiding this comment

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

Highlighting for review as these will be tweaked based on prior discussion.

@microbit-matt-hillsdon
Copy link

I've tried for an alternative on #35

Mainly focussed on the layer we do this at but I've also made some incidental changes to the error code/text.

@microbit-matt-hillsdon
Copy link

Merged the alternative on #35
Closing this one

@microbit-matt-hillsdon microbit-matt-hillsdon deleted the version-warning branch May 23, 2024 13:11
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