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

Run mvn site -B -e -X --projects 'jsgenerator-core' only when the code is merged on main. #255

Closed
FanJups opened this issue Aug 15, 2023 · 11 comments · Fixed by #257
Closed
Labels
CI / CD Pipeline Continuous Integration / Continuous Delivery - Deployment Pipeline done This ticket is solved good first issue Good for newcomers jsgenerator Related to the whole Project jsgenerator-core core module

Comments

@FanJups
Copy link
Member

FanJups commented Aug 15, 2023

Currently, every PR triggers the deployment of GitHub Pages. That's not a good behaviour.

Tip: create another yaml file javadoc.yml with this config and update .github/workflows/maven.yml accordingly

name: Maven Site

on:
  push:
    branches: [ main ]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v3
    - name: Set up JDK 17
      uses: actions/setup-java@v3
      with:
        distribution: 'temurin'
        java-version: 17
        # Given the fact that this is a multimodule project, build process will take long time so we activate caching
        # To know more: https://maven.apache.org/extensions/maven-build-cache-extension/cache.html
        cache: 'maven'
    - name: Build Javadoc Site with Maven
    #To see the full stack trace of the errors, re-run Maven with the -e switch.
    #Re-run Maven using the -X switch to enable full debug logging.
    # -B,--batch-mode Run in non-interactive (batch) mode (disables output color)
    # To learn more about options: https://maven.apache.org/ref/3.6.3/maven-embedder/cli.html
      run: |
        mvn site -B -e -X --projects 'jsgenerator-core'
      env:
        MAVEN_SITE_GITHUB_OAUTH_TOKEN: ${{ secrets.MAVEN_SITE_GITHUB_OAUTH_TOKEN }}
@FanJups FanJups added good first issue Good for newcomers jsgenerator-core core module jsgenerator Related to the whole Project todo Ticket that has to be done labels Aug 15, 2023
@FanJups
Copy link
Member Author

FanJups commented Aug 15, 2023

@shrutikumar15, maybe this will interest you.

@FanJups FanJups added the CI / CD Pipeline Continuous Integration / Continuous Delivery - Deployment Pipeline label Aug 17, 2023
@shrutikumar15
Copy link
Contributor

Do you want to remove this part of the maven.yml file

pull_request_target:
    types: [ opened, edited, synchronize, ready_for_review, review_requested, review_request_removed ]
    branches: [ main ] 

I did not understand why we need two files javadoc.yml and maven.yml

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

Do you want to remove this part of the maven.yml file

pull_request_target:
    types: [ opened, edited, synchronize, ready_for_review, review_requested, review_request_removed ]
    branches: [ main ] 

I did not understand why we need two files javadoc.yml and maven.yml

I am trying to run "clean package" ( or any goal executing test ) for PR and branch main but "site" is only for main since PR is not yet ready.

I think the easiest way to achieve that is using 2 files but maybe I'm wrong. What do you think ?

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

Do you want to remove this part of the maven.yml file

pull_request_target:
    types: [ opened, edited, synchronize, ready_for_review, review_requested, review_request_removed ]
    branches: [ main ] 

I did not understand why we need two files javadoc.yml and maven.yml

No need to remove this part inside maven.yml because we want to run clean package for PR and main

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

Currently, every PR triggers the deployment of GitHub Pages. That's not a good behaviour.

Tip: create another yaml file javadoc.yml with this config and update .github/workflows/maven.yml accordingly

name: Maven Site

on:
  push:
    branches: [ main ]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v3
    - name: Set up JDK 17
      uses: actions/setup-java@v3
      with:
        distribution: 'temurin'
        java-version: 17
        # Given the fact that this is a multimodule project, build process will take long time so we activate caching
        # To know more: https://maven.apache.org/extensions/maven-build-cache-extension/cache.html
        cache: 'maven'
    - name: Build Javadoc Site with Maven
    #To see the full stack trace of the errors, re-run Maven with the -e switch.
    #Re-run Maven using the -X switch to enable full debug logging.
    # -B,--batch-mode Run in non-interactive (batch) mode (disables output color)
    # To learn more about options: https://maven.apache.org/ref/3.6.3/maven-embedder/cli.html
      run: |
        mvn site -B -e -X --projects 'jsgenerator-core'
      env:
        MAVEN_SITE_GITHUB_OAUTH_TOKEN: ${{ secrets.MAVEN_SITE_GITHUB_OAUTH_TOKEN }}

We should only remove mvn site and env from maven.yml

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

maven.yml will look like this

# This workflow will build a Java project with Maven
# For more information see: https://help.github.com/actions/language-and-framework-guides/building-and-testing-java-with-maven

name: Java CI with Maven

on:
  push:
    branches: [ main ]
  pull_request_target:
    types: [ opened, edited, synchronize, ready_for_review, review_requested, review_request_removed ]
    branches: [ main ]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v3
    - name: Set up JDK 17
      uses: actions/setup-java@v3
      with:
        distribution: 'temurin'
        java-version: 17
        # Given the fact that this is a multimodule project, build process will take long time so we activate caching
        # To know more: https://maven.apache.org/extensions/maven-build-cache-extension/cache.html
        cache: 'maven'
    - name: Build with Maven
    #To see the full stack trace of the errors, re-run Maven with the -e switch.
    #Re-run Maven using the -X switch to enable full debug logging.
    # -B,--batch-mode Run in non-interactive (batch) mode (disables output color)
    # To learn more about options: https://maven.apache.org/ref/3.6.3/maven-embedder/cli.html
      run: |
        mvn package -B -e -X

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

Do you want to remove this part of the maven.yml file

pull_request_target:
    types: [ opened, edited, synchronize, ready_for_review, review_requested, review_request_removed ]
    branches: [ main ] 

I did not understand why we need two files javadoc.yml and maven.yml

To sum up, javadoc.yml is only for main and maven.yml is for PR and main.

When a PR is created, we only test the code. But once it is merged, we test through maven.yml and generate site through javadoc.yml
Every change is not made through PR, some safe changes are done directly on main. As an illustration, 2 days ago I updated README on main then test & site were triggered.

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

Current Behaviour :

Every PR runs package and site goals. As a result, GitHub Pages are deployed for every PR because of site goal.

Expected behaviour:

Every new PR should only run package goal.
Every Merged PR and push should run package and site goals.

@FanJups
Copy link
Member Author

FanJups commented Aug 17, 2023

I am also thinking 🤔, maybe test goal should be enough ? package goal seems to do extra work we don't need now. Maybe, it will be useful in the future but now looks like its useless. What do you think about replacing package with test ?

Adding clean will be a plus:

mvn clean site
mvn clean test

@FanJups
Copy link
Member Author

FanJups commented Aug 25, 2023

@shrutikumar15 , please do you need more explanation ?

@shrutikumar15
Copy link
Contributor

@FanJups Sorry I did not see your comments, I have raised a PR - #257

@FanJups FanJups added work in progress At least one person is currently working on this ticket and removed todo Ticket that has to be done labels Aug 27, 2023
FanJups added a commit that referenced this issue Sep 4, 2023
* Added javadoc.yml

* Made changes to javadoc.yml

* Make push events on main to automatically trigger this workflow

---------

Co-authored-by: Fanon Jupkwo <[email protected]>
@FanJups FanJups added done This ticket is solved and removed work in progress At least one person is currently working on this ticket labels Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI / CD Pipeline Continuous Integration / Continuous Delivery - Deployment Pipeline done This ticket is solved good first issue Good for newcomers jsgenerator Related to the whole Project jsgenerator-core core module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants