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

Refactor project config template engine #41

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

j3k0
Copy link
Contributor

@j3k0 j3k0 commented Sep 23, 2021

  • Adds the option to add separators within the ${...} variables.
  • More ambitious goal I have in mind would be to support conditionals (will discuss this in an issue).

 - Adds the option to add separators within the `${...}` variables.
 - More ambitious goal I have in mind would be to support conditionals.
@j3k0
Copy link
Contributor Author

j3k0 commented Oct 7, 2021

Note, this PR was just doing some cleanup. This stuff was like copy-pasted in all different MergeProcesses.

				// Read content and inject any config parameters
				var infoAdditionsProjectContent:String = FileUtils.readFileContentAsString( infoAdditionsProjectFile );
				for (var name:String in APM.config.projectDefinition.configuration)
				{
					var regex:RegExp = new RegExp( "\\$\\{" + name + "\\}", "g" );
					var value:String = APM.config.projectDefinition.getConfigurationParam( name );
					
					infoAdditionsProjectContent = infoAdditionsProjectContent.replace( regex, value );
				}
				
				infoAdditionsProjectContent = infoAdditionsProjectContent.replace( /\$\{applicationId\}/g, APM.config.projectDefinition.applicationId );
				
				FileUtils.writeStringAsFileContent( infoAdditionsFile, infoAdditionsProjectContent );

This change was to have all that code in a single place, a helper class that implements the config-file templating engine.

Now there are some conflicts, but I'll solve them if you agree it's a valid change.

@marchbold
Copy link
Contributor

I think our main concern with this one was just the naming of the class Template, I thought I mentioned it here, but must have been in a discussion. If you can clean it up and move it to something like "ConfigurationMerger" or something.

@github-actions
Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the stale label Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants