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

Update nvika #4848

Merged
merged 2 commits into from
Oct 28, 2021
Merged

Update nvika #4848

merged 2 commits into from
Oct 28, 2021

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Oct 28, 2021

PRing prior to merge of #4847, to see if it works. This should fail CI initially.

@smoogipoo
Copy link
Contributor Author

Don't merge, nvika / one package they're using is borked, and I can repro locally.

@bdach
Copy link
Collaborator

bdach commented Oct 28, 2021

This won't fail CI now, because #4847 is merged, and even though this PR's branch is behind master and doesn't have that merge commit, CI runs not on this branch, but on the merge commit of this branch to master.

That said I've tested the fix yesterday and it worked correctly locally.

@smoogipoo
Copy link
Contributor Author

smoogipoo commented Oct 28, 2021

Unfortunate... I'd have preferred to touch the mentioned PR myself to avoid this.

Can be merged then.

@smoogipoo smoogipoo removed the blocked label Oct 28, 2021
@bdach
Copy link
Collaborator

bdach commented Oct 28, 2021

I forgot that there's actually another option to view the failures, namely they will show on your fork's CI because that just builds the branch as given.

The failures are visible here: https://github.com/smoogipoo/osu-framework/actions/runs/1393015552

@smoogipoo
Copy link
Contributor Author

Amazing. The annotations will be of more help it seems 😅

@peppy peppy enabled auto-merge October 28, 2021 05:23
@peppy peppy merged commit fee11a7 into ppy:master Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants