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

NIFI-1850 - JSON-to-JSON Schema Converter Editor #511

Closed
wants to merge 1 commit into from

Conversation

YolandaMDavis
Copy link
Contributor

This is a merge from 0.7 of the Json-to-Json (Jolt) Editor with refactoring for masterless clustering and bower dependency support.

.validateErrors(processor.getValidationErrors()).build();
}

private Map<String,ComponentDescriptor> getComponentDescriptors(final ProcessorConfigDTO processorConfig){
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it would be more appropriate to name the operation buildComponentDescriptors(..) instead of get*?
IMHO get* usually implies an operation that is returning something that already exist. Your call, but wanted to share.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@olegz makes sense to me I can change this

@olegz
Copy link
Contributor

olegz commented Jun 15, 2016

@YolandaMDavis While I believe it looks good, I think someone from the web team has to look at it as well

@YolandaMDavis
Copy link
Contributor Author

@olegz thanks for taking a look! as noted above I'll make the changes cited and update this request.

@@ -355,10 +355,5 @@
<artifactId>spock-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this dependency no longer used or does this PR make it unnecessary now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm I didn't directly remove this perhaps odd merge during rebase? Spock tests are still functioning however I will restore it just in case.

@mcgilman
Copy link
Contributor

@YolandaMDavis While reading over your PR I noticed that the ControllerService and ReportingTask URI's in the StandandNiFiWebConfigurationContext are incorrect. They shouldn't contain the path segment /node anymore. Could you fix that in your PR?

@YolandaMDavis
Copy link
Contributor Author

@mcgilman Thank you for reviewing! The PR has been updated based on your comments, including the corrections in the path segment for StandardNiFiWebConfigurationContext. I also found that same path in ComponentStateEndointMerger as part of a regex pattern which I corrected as well.

…merge from 0.7.0 - refactor for masterless cluster)
@mcgilman
Copy link
Contributor

Looks great @YolandaMDavis

I'm merged this into master. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants