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

Fixes #12905: Create validation workflows plugins #61

Conversation

fanf
Copy link
Member

@fanf fanf commented Jul 8, 2018

@fanf
Copy link
Member Author

fanf commented Jul 18, 2018

PR rebased

@fanf fanf force-pushed the arch_12905/create_validation_workflows_plugins branch from a66bccb to acd155c Compare July 18, 2018 08:39
@fanf
Copy link
Member Author

fanf commented Jul 18, 2018

PR rebased

@fanf fanf force-pushed the arch_12905/create_validation_workflows_plugins branch from acd155c to fb9f562 Compare July 18, 2018 14:53
@fanf
Copy link
Member Author

fanf commented Jul 18, 2018

PR rebased

@fanf fanf force-pushed the arch_12905/create_validation_workflows_plugins branch from fb9f562 to b06746c Compare July 18, 2018 16:00
Copy link
Member

@VinceMacBuche VinceMacBuche left a comment

Choose a reason for hiding this comment

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

changes-validation/src/test/scala/com/normation/plugins/changesvalidation/placeholder can be removed!


include ../makefiles/common-scala-plugin.mk

target/$(NAME)/changes-validation-schema.sql:
Copy link
Member

Choose a reason for hiding this comment

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

The file is not present and will prevent build of the plugin for now

*************************************************************************************
*/

/* add you plugin css in that file */
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to keep that file ?

I don't think so and i would remove it and remove it from the html

*************************************************************************************
*/

/* add you plugin JS content here */
Copy link
Member

Choose a reason for hiding this comment

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

Sama as css file

<div id="plugin-main" data-lift="surround?with=common-layout;at=content">
<head_merge>
<title>Rudder :: Changes Validation</title>
<link rel="stylesheet" type="text/css" href="/toserve/changesvalidation/changes-validation.css" media="screen" data-lift="with-cached-resource">
Copy link
Member

Choose a reason for hiding this comment

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

I think it could be removed and line below too (see comment on css file)

case Some(cr) => changeRequestChangesSerialisation.serialise(cr)
case None => Full(NodeSeq.Empty)
}
if(xml.toString().contains(code)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we ckeck we are in production or in dev run mode of Lift ? to prevent enabling it by default on production :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, we don't really want to keep it in the released version :)

main-build.conf Outdated
parent-plugin=1.3
parent-plugin-version=${rudder-branch}-${parent-plugin}

lib-common=1.2
Copy link
Member

Choose a reason for hiding this comment

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

can be removed

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder-plugins/pull/61
-- Your faithful QA

1 similar comment
@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder-plugins/pull/61
-- Your faithful QA

Copy link
Member

@VinceMacBuche VinceMacBuche left a comment

Choose a reason for hiding this comment

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

Miss click between approve and request change ... i prevent the merge with labels ....

@fanf fanf force-pushed the arch_12905/create_validation_workflows_plugins branch from b06746c to 5e713b6 Compare July 23, 2018 20:10
@fanf
Copy link
Member Author

fanf commented Jul 23, 2018

PR rebased

@VinceMacBuche
Copy link
Member

It misses the sql file in src main resources, makefile will fail

@VinceMacBuche
Copy link
Member

or remove the makefile entry

@fanf fanf force-pushed the arch_12905/create_validation_workflows_plugins branch from 5e713b6 to d7d0f24 Compare July 25, 2018 11:07
@fanf
Copy link
Member Author

fanf commented Jul 25, 2018

PR rebased

@Normation-Quality-Assistant
Copy link
Contributor

OK, merging this PR

@Normation-Quality-Assistant Normation-Quality-Assistant merged commit d7d0f24 into Normation:master Jul 25, 2018
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