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

FeatureRequest: Allow Parameters from settings.json file for compile functions #3428

Open
ChrisBlankDe opened this issue Mar 18, 2024 · 5 comments
Assignees

Comments

@ChrisBlankDe
Copy link
Contributor

Describe the issue
We as well as other partners use the settings from the settings.json in their build pipelines.
Instead of having to parse the settings ourselves, we could implement this option directly in the corresponding cmdlets as an alternate parameter set.

This should be implemented for Compile-AppInBcContainer and Compile-AppWithBcCompilerFolder.

Params whe can use from settings.json:

  • $assemblyProbingPaths
  • $GenerateCrossReferences
  • $rulesetFile
  • $enableExternalRulesets
  • $EnableCodeCop
  • $EnableAppSourceCop
  • $EnablePerTenantExtensionCop
  • $EnableUICop
  • $GenerateReportLayout

I could provide a pull request. But I would like to clarify first so that I don't have to do any unnecessary work.

What do you think about this?

@freddydk
Copy link
Contributor

Is this the settings from VS Code?
Would this typically be checked into the repository?

@ChrisBlankDe
Copy link
Contributor Author

ChrisBlankDe commented Mar 19, 2024

yes. just the casual settings.json file in .vscode directory.

we check it into git to provide every dev the same experience (same cops, ruleset and compile options).

@ChrisBlankDe ChrisBlankDe changed the title FeatureRequest: Allow Parameters from setting.json file for compile functions FeatureRequest: Allow Parameters from settings.json file for compile functions Mar 19, 2024
@freddydk
Copy link
Contributor

Got it.

How would you determine who wins? Parameters or settings? What is your suggested implementation?

And what about AL-Go - would we support the same there?

@ChrisBlankDe
Copy link
Contributor Author

on Implantation i see three possibilities:

1. Use two parameter sets
Compile-AppInBcContainer -EnableCodeCop -rulesetFile [String] [...]
Compile-AppInBcContainer -FromVsCodeSettings
If you use FromVsCodeSettings you can not define any of the other params. Take all from settings, or none.
--> not so flexible but very clear and easy. not breaking

2. Load Settings as default
Compile-AppInBcContainer -EnableCodeCop -rulesetFile [String] [...]
Look for settings file and use as default value. overwriting values is possible by using a parameter. PowerShell Parameter always wins.
--> very flexible but might break existing implementations

3. Use an Additional Parameter
Compile-AppInBcContainer -FromVsCodeSettings -EnableCodeCop -rulesetFile [String] [...]
Use the vscode settings but allow overwriting them with another parameter. PowerShell Parameter always wins.
--> very flexible and does not break anything. My Fav.

I think option two is not a real option :)

for the sake of completeness, it must also be said that we can only use default .vscode/settings.json here. i do not see any possibility for the support of multi-root workspace files.

And what about AL-Go - would we support the same there?

Hard to tell since i do not use AL-Go on daily base, just playing around...
as i understand it, different app types (and configurations) are much more separated from each other in al-go. we even have different templates for appsource apps and ptes.
also in al-go we already have config file driven settings.

@freddydk
Copy link
Contributor

3 would also be my favorite.
Two more questions.

How would you determine from where to load the vs code settings file?
Would the same implementation make sense in Compile-AppWithCompilerFolder? (containerless compilation)

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

No branches or pull requests

2 participants