NEST-321: Create Media Theme deployment #2
Conversation
| Justification = "It's a console application it doesn't need localization.")] | ||
| public static void Main(string[] args) | ||
| { | ||
| if (args.Length == 3 && bool.TryParse(args[2], out bool clearMediaFolder)) |
There was a problem hiding this comment.
I think it'd be better and more future-proof to use a library for parsing CLI. See Hastlayer.
There was a problem hiding this comment.
Could you please send me a specific example? I couldn't find any.
| { | ||
| // Creating directory for the deployment. | ||
| var currentRootDirectory = Directory.GetDirectoryRoot(Directory.GetCurrentDirectory()); | ||
| var newDirectoryPath = Path.Join(currentRootDirectory, "MediaThemeDeployment_") |
There was a problem hiding this comment.
It should accept a parameter for the target directory. It can fall back to this one if it's not given.
| // Determine whether the directory exists. | ||
| if (Directory.Exists(newDirectoryPath)) | ||
| { | ||
| WriteLine("That file already exists."); |
| { | ||
| dynamic assetJObject = new JObject(); | ||
| assetJObject.SourcePath = Path.Join( | ||
| "_MediaTheme/Assets", |
There was a problem hiding this comment.
Use these texts as constants as I did in the module. For all strings.
| // Getting templates. | ||
| var pathToTemplates = Path.Join(pathToTheme, "Views"); | ||
|
|
||
| var allTemplatesPaths = Directory |
There was a problem hiding this comment.
When copying the files and adding them to the recipe, please do a string replace in the filename: change . to _ and - to __.
| "Globalization", | ||
| "CA1303:Do not pass literals as localized parameters", | ||
| Justification = "It's a console application it doesn't need localization.")] | ||
| public static void Main(string[] args) |
There was a problem hiding this comment.
For such a huge method I'd recommend creating private methods for clean code.
| { | ||
| public const string MediaThemeRootDirectory = "_MediaTheme"; | ||
| public const string MediaThemeDeploymentDirectory = "MediaThemeDeployment_"; | ||
| public const string MediaThemeWwwRootDirectory = "wwwroot"; |
There was a problem hiding this comment.
| public const string MediaThemeWwwRootDirectory = "wwwroot"; | |
| public const string LocalThemeWwwRootDirectory = "wwwroot"; |
| public const string MediaThemeRootDirectory = "_MediaTheme"; | ||
| public const string MediaThemeDeploymentDirectory = "MediaThemeDeployment_"; | ||
| public const string MediaThemeWwwRootDirectory = "wwwroot"; | ||
| public const string MediaThemeViewsDirectory = "Views"; |
There was a problem hiding this comment.
| public const string MediaThemeViewsDirectory = "Views"; | |
| public const string LocalThemeViewsDirectory = "Views"; |
| public const string MediaThemeDeploymentDirectory = "MediaThemeDeployment_"; | ||
| public const string MediaThemeWwwRootDirectory = "wwwroot"; | ||
| public const string MediaThemeViewsDirectory = "Views"; | ||
| public const string MediaThemeTemplatesFolder = "Templates"; |
There was a problem hiding this comment.
| public const string MediaThemeTemplatesFolder = "Templates"; | |
| public const string MediaThemeTemplatesDirectory = "Templates"; |
| [Option('i', "default-id", Required = true, HelpText = "Default theme ID.")] | ||
| public string? DefaultThemeId { get; set; } |
There was a problem hiding this comment.
Isn't it base theme instead of default theme?
Co-authored-by: Márk Bartha <[email protected]>
NEST-321