search

UFRN-URNAI / urnai-tools

A modular Deep Reinforcement Learning library that supports multiple environments, made with Python 3.6.
Apache License 2.0
5 stars 8 forks source link

Ci/fix lint job #46

Closed sudiptob2 closed 2 years ago

sudiptob2 commented 2 years ago

fixes #44

Observations

Due to the fact that GitHub action does a shallow clone of the repo (using actions/checkout@v3) the checked-out repository does not have a branch master. As a result, it gives fatal: ambiguous argument 'origin/master' error when make lint is run.

Fix

By forcing fetch-depth: 0 we can checkout the entire repo and the origin/master will be available. :blush:

Opinion

I am not getting the point of running make lint on master branch because the code that needs to be checked by the linter are in the current branch. Shouldn't we run the workflow on the origin/current-PR-branch? :pray:

sudiptob2 commented 2 years ago

@alvarofpp can you please review :pray:

sudiptob2 commented 2 years ago

@alvarofpp

I agree with you. Since this is an integration workflow, I think you could remove the push: branches: [ master ].

But even then make lint will run on the master branch and the changed code will remain on the PR branch. I think we should remove 

  pull_request:
    branches: [ master ]
alvarofpp commented 2 years ago

@sudiptob2 The push event will trigger the jobs when a branch is pushed, regardless of which branch it is (the master or the one someone else is working on). The intention would be to trigger only during a Pull Request, that's why I think the pull_request event is ideal, besides that it occurs, by default, in 3 types of activities (opened, synchronize, and reopened). So we would have:

on:
  pull_request:
    branches: [ master ]
sudiptob2 commented 2 years ago

Thanks understood :pray:

alvarofpp commented 2 years ago

@sudiptob2 Thank you for your contribution!