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

Feat/create gradle page plugin #5

Merged
merged 3 commits into from
Feb 20, 2018

Conversation

julienmege
Copy link
Contributor

@julienmege julienmege commented Feb 19, 2018

closes BS-18048

Copy link
Contributor

@abirembaut abirembaut left a comment

Choose a reason for hiding this comment

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

👍
Just a small comment


project.node {
version = '8.9.4'
npmVersion = '5.6.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice to be able to configure the versions of node and npm to use in build.gradle

Copy link

@cpuy cpuy Feb 20, 2018

Choose a reason for hiding this comment

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

I don't know if we can do it out of the box with

node {
            version = '8.9.4'
            npmVersion = '5.6.0'
}

otherwise you need to add an extension for your plugin

Anyway sounds like a better idea to specify this in build.gradle instead of plugin

Copy link
Contributor

@benjaminParisel benjaminParisel left a comment

Choose a reason for hiding this comment

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

LGTM

import org.gradle.api.Plugin
import org.gradle.api.Project

class NpmPagePlugin extends PagePlugin {
Copy link

Choose a reason for hiding this comment

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

ℹ️ This seems useless to me. I believe this should be done in PagePlugin and ReactPagePlugin override implementDistribution

contents {
from('resources') { into '/' }
from('build') {
exclude 'distributions'
Copy link

Choose a reason for hiding this comment

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

ℹ️ You'll have trouble with this approach since some things will be added to build directory (example for tests)

outputs.dirs('node_modules')
}

def buildNpm = project.task( [type: com.moowork.gradle.node.npm.NpmTask, dependsOn: project.tasks.npm_install] ,'buildNpm') {
Copy link

Choose a reason for hiding this comment

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

I would name it 'buildFrontend' or 'buildPage'


def cleanNpm = project.task([:], 'cleanNpm') {
doFirst {
project.delete 'dist'
Copy link

Choose a reason for hiding this comment

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

will not fit with ReactPlugin

@cpuy
Copy link

cpuy commented Feb 20, 2018

Overall I think that we need a plugin extension to configure:

  • nodeVersion
  • npmVersion
  • pageDir (i.e. frontend build dir or something)

@cpuy
Copy link

cpuy commented Feb 20, 2018

Although I think the plugin is a good idea I would rather make it work first with a standard gradle build, then improve it by implementing a plugin.

Anyway this should definitively be used by ici module and published in bonita-dev-tools

Furthermore, keep in mind that since react-script build in build folder we will always have an issue with that since it is the same folder used by gradle. We might want to find a solution for that first (i.e. configure the react-script output folder)

@cpuy
Copy link

cpuy commented Feb 20, 2018

See facebook/create-react-app#1354

@benjaminParisel benjaminParisel merged commit dd3ca7f into dev Feb 20, 2018
@benjaminParisel benjaminParisel deleted the feat/create_gradle_page_plugin branch February 20, 2018 16:39
bonita-ci pushed a commit that referenced this pull request Jun 23, 2022
* Use processId filter from url
* add tests
* Update page version

covers [RUNTIME-1011](https://bonitasoft.atlassian.net/browse/RUNTIME-1011)
bonita-ci pushed a commit that referenced this pull request Mar 14, 2024
* Use processId filter from url
* add tests
* Update page version

covers [RUNTIME-1011](https://bonitasoft.atlassian.net/browse/RUNTIME-1011)
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.

4 participants