Skip to content

Conversation

@nikithauc
Copy link
Contributor

For beta version, the typescript typings top-level namespace is 'microsoftgraphbeta'. The existing code has main namespaces always set to 'microsoftgraph'. This PR brings in the following changes-

  1. For the main namespace to be generated as 'microsoftgraphbeta' when generating beta version, we will be passing "typescript.namespacePostfix:beta" as a property '-p typescript.namespacePostfix:beta'.
  2. During the generation of the typescript main namespace, the code will if the property 'typescript.namespacePostfix' exists and if present, will append the value to "microsoftgraph".

public string GetMainNamespace()
{
return TypeScriptMainNamespaceName;
return TypeScriptMainNamespaceNamePrefix + (ConfigurationService.Settings.Properties != null && ConfigurationService.Settings.Properties.ContainsKey("typescript.namespacePostfix") ? ConfigurationService.Settings.Properties["typescript.namespacePostfix"] : string.Empty);
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Please consider splitting this into multiple lines. And if you use null-conditional operator:

Suggested change
return TypeScriptMainNamespaceNamePrefix + (ConfigurationService.Settings.Properties != null && ConfigurationService.Settings.Properties.ContainsKey("typescript.namespacePostfix") ? ConfigurationService.Settings.Properties["typescript.namespacePostfix"] : string.Empty);
var properties = ConfigurationService.Settings?.Properties;
return properties?.ContainsKey("typescript.namespacePostfix") == true
? TypeScriptMainNamespaceNamePrefix + properties["typescript.namespacePostfix"]
: TypeScriptMainNamespaceNamePrefix;

// Arrange
var languageStr = language.ToString();
var directoryPostfix = isPhpBeta ? "PHPBeta" : languageStr;
var directoryPostfix = languageStr + (isBeta ? "Beta" : string.Empty);
Copy link

Choose a reason for hiding this comment

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

not sure if case sensitivity matters here but for php would languageStr be "PHP" so the string ends up exactly the same. Since it's a passed in parameter i don't know that's guaranteed and if it matters.

Copy link

Choose a reason for hiding this comment

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

i think this is answered looking at how it's called from the PHPMultipleNamespaces file.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is fed from an enum that is local to the tests, so nobody will be able to change it by changing a string parameter somewhere. The test caller needs to specify an enum value.

Also this is only used as part of a directory name where case doesn't matter as long as you are on Windows :)

@nikithauc nikithauc merged commit 6e9b342 into dev Sep 10, 2020
@baywet baywet deleted the nikithauc/typescript-beta-mainnamespace branch March 8, 2022 14:30
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.

4 participants