odin-bot-v2 icon indicating copy to clipboard operation
odin-bot-v2 copied to clipboard

Ensure CircleCI tests get run for PR's from forks

Open Asartea opened this issue 1 year ago • 9 comments

Complete the following REQUIRED checkboxes:

  • [x] I have thoroughly read and understand The Odin Project Contributing Guide

  • [x] The title of this issue follows the command name: brief description of request format, e.g. /help: add optional @user parameter

The following checkbox is OPTIONAL:

  • [ ] I would like to be assigned this issue to work on it

1. Description of the Feature Request: Yesterday a broken change got into the bot's main branch (I had forgotten to push a snapshot update), which caused it to not update to new versions of commands. CircleCI caught this, but it only ran once the change landed on the main branch, at which point it was already in. It should have also run when I proposed the PR, so that it can be caught before it gets merged

2. Acceptance Criteria:

  • [ ] CircleCi runs for every PR, not just PR's from branches on this repo

3. Additional Information:

Asartea avatar Dec 01 '23 11:12 Asartea

@Asartea Thanks, shared with the team.

01zulfi avatar Feb 24 '24 08:02 01zulfi

poke

Asartea avatar Jul 01 '24 16:07 Asartea

@01zulfi Do you need help with this?

CouchofTomato avatar Jul 01 '24 20:07 CouchofTomato

@CouchofTomato Yeah, I don't think I have access to TOP's CircleCI.

Looking back at Discord chats, CircleCI workflows should trigger from forks, but I guess that's not the case with https://github.com/TheOdinProject/odin-bot-v2/pull/556

@KevinMulhern can you see what's up

Discord chat: https://discord.com/channels/505093832157691914/817034720881672233/1210869220645015573

01zulfi avatar Jul 02 '24 06:07 01zulfi

I did check circle CI and it is setup for being triggered from forks so it's a weird one. Hopefully Kevin can get to the bottom of it.

CouchofTomato avatar Jul 03 '24 12:07 CouchofTomato

Weirdly enough, in #556 it did run after the second commit. Testing...

Asartea avatar Jul 03 '24 13:07 Asartea

And in #561 it immediately ran a test. Going to blame it on "Vague weirdness"

Oh wait I see: CircleCI has now picked it up under my account, not under TOP's

Asartea avatar Jul 03 '24 13:07 Asartea

@Asartea just double checking that this issue can be closed?

MaoShizhong avatar Aug 07 '24 00:08 MaoShizhong

I don't think has actually been fixed? Its fixed for the specific case of 'Asartea submits a PR', but that's because I have my own account on CircleCi, so its running tests under my account

Asartea avatar Aug 07 '24 06:08 Asartea