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

Adding repo owner input #2774

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Adding repo owner input #2774

merged 4 commits into from
Oct 11, 2024

Conversation

rmoreliovlabs
Copy link
Contributor

@rmoreliovlabs rmoreliovlabs commented Sep 30, 2024

Description

Right now, checks for pull requests (PRs) from external contributors using forked repositories are not passing. The main issue is that the rootstock-integration-tests workflow is attempting to find the branch in the original rsksmart/rskj repository instead of the contributor's fork. Here are the specific problems we've encountered:

Git Checkout Error: error: pathspec 'vovchyk/status-badge' did not match any file(s) known to git
and

SonarQube Execution Error: Execution failed for task '
'.Project not found. Please check the 'sonar.projectKey' and 'sonar.organization' properties, the 'SONAR_TOKEN' environment variable, or contact the project administrator to check the permissions of the user the token belongs to.

It appears that the SonarQube check is being triggered automatically for external PRs of branches from forked repos, which should not happen due to potential access issues

Implementation

Updated rit.yml Workflow:
Added the repo-owner input to the Rootstock Integration Tests action to ensure it correctly clones the repository based on the PR's owner:

- name: Run Rootstock Integration Tests
  uses: rmoreliovlabs/rootstock-integration-tests@feature/add_repo_owner_input
  with:
    rskj-branch: ${{ env.RSKJ_BRANCH }}
    powpeg-node-branch: ${{ env.POWPEG_BRANCH }}
    rit-branch: ${{ env.RIT_BRANCH }}
    repo-owner: ${{ github.event.pull_request.head.repo.owner.login || github.repository_owner }}

Conditionally Trigger SonarQube Check:

Implemented logic to skip the SonarQube check for PRs from external contributors. This prevents unnecessary execution that could fail due to access issues.
if [ "$GH_EVENT" = pull_request ] && [ "${{ github.event.pull_request.head.repo.fork }}" != "true" ]; then

How Has This Been Tested?

I created two branches in my forked repositories for rskj and rootstock-integration-tests to test my changes. I verified that everything works correctly in the forked repos. Here are the pull requests:

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@rmoreliovlabs rmoreliovlabs marked this pull request as ready for review September 30, 2024 15:12
Copy link
Contributor

@fmacleal fmacleal left a comment

Choose a reason for hiding this comment

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

Good job, the fix are simple and straight to the point. Well done!

I just have a few comments just to keep the solution following same principles to deal with the situations when to run or not the pipeline or when we are configuring and using github variables.

.github/workflows/rit.yml Outdated Show resolved Hide resolved
.github/workflows/build_and_test.yml Show resolved Hide resolved
.github/workflows/rit.yml Outdated Show resolved Hide resolved
fmacleal
fmacleal previously approved these changes Oct 2, 2024
Copy link
Contributor

@fmacleal fmacleal left a comment

Choose a reason for hiding this comment

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

Good job!

@rmoreliovlabs rmoreliovlabs force-pushed the feature/adding_repo_owner_input branch from 3d92a19 to 9f433cf Compare October 11, 2024 14:39
Copy link
Contributor

@fmacleal fmacleal left a comment

Choose a reason for hiding this comment

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

LGTM. Good Job!

Copy link

@Vovchyk Vovchyk merged commit d64a6bc into master Oct 11, 2024
11 checks passed
@Vovchyk Vovchyk deleted the feature/adding_repo_owner_input branch October 11, 2024 15:04
@aeidelman aeidelman added this to the Arrowhead 6.4.0 milestone Dec 4, 2024
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