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

issue/3017 Added config.json:build.strictMode #3018

Merged
merged 2 commits into from
Jan 14, 2021
Merged

issue/3017 Added config.json:build.strictMode #3018

merged 2 commits into from
Jan 14, 2021

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented Jan 14, 2021

fixes #3017

Added

  • config.json:build.strictMode to turn on strict mode directives in javascript output
  • Turned on in standard course

@moloko
Copy link
Contributor

moloko commented Jan 14, 2021

this will also need to be added to the schema for config.json I think? and also suggest adding it to the 'OOTB' course's config.json

I'm thinking that for FW courses strict mode should be on by default but maybe off by default for AAT ones? purely on the basis that an AAT instance is more likely to have a large collection of 3rd party plugins and it's unreasonable to require an another to amend this setting on every single course they have in the AAT just because one or two of the plugins are missing variable declarations

@oliverfoster
Copy link
Member Author

Yup, sounds like a plan. Need to make sure it fixes the issue before I do that stuff.

@moloko
Copy link
Contributor

moloko commented Jan 14, 2021

@oliverfoster it's certainly fixing the problem of using a 'rollup' build of the FW with spoor v3.3.2 - not seeing any errors now if I set strictMode: false

@oliverfoster
Copy link
Member Author

oliverfoster commented Jan 14, 2021

There isn't any build property in the config schema atm. Is that possible / allowed? @taylortom @tomgreenfield
Should I move it elsewhere?

@oliverfoster
Copy link
Member Author

oliverfoster commented Jan 14, 2021

@moloko it might just be easier to turn off the strict mode universally until some later breaking-change date?

@moloko
Copy link
Contributor

moloko commented Jan 14, 2021

sure, or just default it to off unless enabled via the setting in config.json - I think most people developing plugins do so in the FW first so that would give them the opportunity to get their code working to the strict requirement. I'd still prefer it to be enabled by default for the OOTB course (via config.json)

@moloko
Copy link
Contributor

moloko commented Jan 14, 2021

could we change the name of the setting in config.json to strictMode?

@oliverfoster
Copy link
Member Author

oliverfoster commented Jan 14, 2021

So strict mode is off by default (AAT), enabled with config.json:build.strictMode = true; and enabled in the standard course (FW).

@oliverfoster oliverfoster changed the title issue/3017 Added config.json:build.strict issue/3017 Added config.json:build.strictMode Jan 14, 2021
@oliverfoster
Copy link
Member Author

oliverfoster commented Jan 14, 2021

@davetaz :

I can confirm that works and the course builds from the AAT with my broken plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@moloko moloko merged commit ae17816 into master Jan 14, 2021
@moloko moloko deleted the issue/3017 branch January 14, 2021 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adapt_framework: Remove strict mode
4 participants