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

Add method support #381

Closed
wants to merge 7 commits into from
Closed

Add method support #381

wants to merge 7 commits into from

Conversation

dprzybyl
Copy link
Collaborator

No description provided.

# Conflicts:
#	app/aem/core/src/main/kotlin/com/cognifide/apm/core/grammar/argument/ArgumentResolver.kt
#	app/aem/core/src/test/groovy/com/cognifide/apm/core/grammar/ArgumentResolverTest.groovy
#	app/aem/core/src/test/groovy/com/cognifide/apm/core/grammar/ScriptRunnerTest.groovy
#	app/aem/core/src/test/resources/define.apm
methodDefinitions.put(methodDefinition.getName(), methodDefinition);
}

protected final void unbindMethodDefinition(MethodDefinition methodDefinition) {

Choose a reason for hiding this comment

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

hmmm unbind method will be called by name convention? or should be annotated as well?

Resource resource = resourceResolver.getResource(path);
List<ApmString> values = StreamSupport.stream(resource.getChildren().spliterator(), false)
.filter(this::isValidResource)
.map(Resource::getName)

Choose a reason for hiding this comment

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

how about accepting optionally nameRegex as second parameter?
I believe checking ":" will be too naive and must be configurable

for (Resource market : resource.getChildren()) {
if (isPageResource(market)) {
Map<String, ApmType> value = new HashMap<>();
value.put("code", new ApmString(market.getName()));
Copy link

Choose a reason for hiding this comment

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

code? please ensure to use here strict Adobe/AEM nomenclature / a name typically used by Adobe to describe nodes at that level

Copy link

Choose a reason for hiding this comment

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

if nodes with en_US (another form of structuring AEM content) will be detected how about decomposing them to "en" and "US on the fly to provide more useful data for APM scripting?

@@ -35,20 +35,23 @@ import com.cognifide.apm.core.grammar.utils.ImportScript
import com.cognifide.apm.core.grammar.utils.RequiredVariablesChecker
import com.cognifide.apm.core.logger.Position
import com.cognifide.apm.core.logger.Progress
import com.cognifide.apm.core.method.MethodInvoker

Choose a reason for hiding this comment

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

mixing kt with java is a mess, are you planning to remove kt ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I'm planning that

@dprzybyl dprzybyl closed this Oct 1, 2023
@dprzybyl dprzybyl deleted the add-method-support branch October 2, 2023 19:38
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.

2 participants