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.

Sign up for GitHub

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

Jump to bottom

Experimenting with the double trigger. #97

Merged
merged 5 commits into from
Jun 26, 2023
Merged

Experimenting with the double trigger. #97

merged 5 commits into from
Jun 26, 2023

Conversation

SeriousHorncat
Copy link
Collaborator

@SeriousHorncat SeriousHorncat commented Jun 22, 2023 •
edited by JmScherer
Loading

Checklist before requesting a review

  • I have performed a self-review of my code.
  • My code follows the style guidelines enforced by static analysis tools.
  • My changes generate no new warnings.

Pull Request Details

  • During CGDS Developer Team drop in hours discussion, the team came to a consensus on how to disable jobs from being executed twice due to running workflows on both the 'push' and 'pull_request' triggers. Need to keep the 'pull_request' trigger due to accepting PRs from forked branches. We found a satisfactory solution from the following ticket Pull request tests run twice? zopefoundation/meta#145 (comment).
  • Tagging the ubuntu build to the current LTS instead of relying on 'ubuntu-latest' by GitHub.

To Review:

  • Static Analysis by Reviewer
  • Observe the changes to the workflows in the GitHub actions of Rosalution repository.
  • All Github Actions checks have passed.

@SeriousHorncat SeriousHorncat self-assigned this Jun 22, 2023
@SeriousHorncat SeriousHorncat added the enhancement New feature or request label Jun 22, 2023
@SeriousHorncat
Tagging the version of ubuntu being used for ubuntu-latest for consis…
b660ca3 
…tentcy.

ubuntu-22.04 is the current long term support (LTS) version.
@JmScherer
Copy link
Collaborator

I can't link it in the PR directly in code, but in the .github/workflows/common-static-analysis.yml file, we modified the triggers to match Quac.

on:
  push:
  pull_request:
    types:
      - opened
      - synchronize
      - reopened
      - ready_for_review
      - converted_to_draft
  workflow_dispatch:

Do we want to change it back to the previous trigger declarations?

on: [push, pull_request, workflow_dispatch]

JmScherer
JmScherer reviewed Jun 23, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
SeriousHorncat and others added 2 commits June 26, 2023 11:59
@SeriousHorncat
Update common-static-analysis.yml
367e1d2 
Signed-off-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
@SeriousHorncat @JmScherer
Apply suggestions from code review
b745bf6 
Co-authored-by: James Scherer <jscherer@uab.edu>
Signed-off-by: Angelina Uno-Antonison <angelina.antonison@gmail.com>
@SeriousHorncat
Copy link
Collaborator Author

Agree with this. I updated and removed it.

SeriousHorncat
SeriousHorncat commented Jun 26, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
@SeriousHorncat
Update README.md
c5d8dbc 
Removed line causing the linting to break.

Signed-off-by: Angelina Uno-Antonison <angelina.antonison@gmail.com>
@JmScherer
Copy link
Collaborator

This PR is good to go!

@SeriousHorncat SeriousHorncat merged commit b431547 into pr_cicd_experiment_status Jun 26, 2023
8 checks passed
@SeriousHorncat SeriousHorncat deleted the experiemtning_to_prevent_double_triggers branch June 26, 2023 18:40
@SeriousHorncat SeriousHorncat mentioned this pull request Jun 26, 2023
Merged
SeriousHorncat added a commit that referenced this pull request Jun 26, 2023
@SeriousHorncat @JmScherer
PR-CI/CD experiment status (#98)
90e17d6 
* Matching quac system test

* Re-enable synchronize

* Experimenting with a conditional to prevent running jobs multiple times
on push and pull requests yet still run for PR's for forks.

* Experimenting with the double trigger. (#97)

* Experimenting with the double trigger.

* Tagging the version of ubuntu being used for ubuntu-latest for consistentcy.
ubuntu-22.04 is the current long term support (LTS) version.

* Update common-static-analysis.yml

Signed-off-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>

* Apply suggestions from code review

Co-authored-by: James Scherer <jscherer@uab.edu>
Signed-off-by: Angelina Uno-Antonison <angelina.antonison@gmail.com>

* Update README.md

Removed line causing the linting to break.

Signed-off-by: Angelina Uno-Antonison <angelina.antonison@gmail.com>

---------

Signed-off-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
Signed-off-by: Angelina Uno-Antonison <angelina.antonison@gmail.com>
Co-authored-by: James Scherer <jscherer@uab.edu>

---------

Signed-off-by: Angelina Uno-Antonison <ange.unoantonison@gmail.com>
Signed-off-by: Angelina Uno-Antonison <angelina.antonison@gmail.com>
Co-authored-by: James Scherer <jscherer@uab.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JmScherer JmScherer JmScherer approved these changes

Assignees

@SeriousHorncat SeriousHorncat

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SeriousHorncat @JmScherer