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

Fix downstream CI and add manually triggered downstream CI test #223

Merged
merged 2 commits into from
Apr 11, 2022

Conversation

unkcpz
Copy link
Member

@unkcpz unkcpz commented Apr 11, 2022

No description provided.

@unkcpz unkcpz requested a review from csadorf April 11, 2022 07:54
@unkcpz
Copy link
Member Author

unkcpz commented Apr 11, 2022

I accidentally push a fix of downstream ci fix (untested) to the master, since as admin we are not limited. The owner permission setting is reset to as owner of the repo we also need to push to unprotected branch first instead of push to master directly.

Copy link
Member

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

I don't think this test should run on every push, should it?

@unkcpz unkcpz force-pushed the fix/205/downstream-ci branch from 0829283 to a0b2032 Compare April 11, 2022 08:17
@unkcpz
Copy link
Member Author

unkcpz commented Apr 11, 2022

I don't think this test should run on every push, should it?

No, It shouldn't, I added it to trigger it manually. I change it back and after this merged, we can go to CI action and trigger the test manually, hopefully?

@unkcpz unkcpz requested a review from csadorf April 11, 2022 09:28
By.XPATH, '//li[@id="tab-key-17" and @class="lm-TabBar-tab p-TabBar-tab"]'
).click() # click `From Examples` for input structure
By.XPATH, '//li[@id="tab-key-17"]'
).click() # click `From Examples` tab for input structure
Copy link
Member

Choose a reason for hiding this comment

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

This looks like an unrelated change. Do you want to merge it with this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is the fix of a previously wrongly pushed commit f2d840d. I test locally this change will pass the downstream CI.

I think we can merge it with this PR and I then specify the details about this commit.

Copy link
Member

Choose a reason for hiding this comment

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

For next time I would recommend to explicitly revert f2d840d and then apply the additional change. You could still do this here and then either rebase and merge or squash merge.

@unkcpz unkcpz requested a review from csadorf April 11, 2022 10:56
@unkcpz unkcpz changed the title Add manually triggered downstream CI test Fix downstream CI and add manually triggered downstream CI test Apr 11, 2022
By.XPATH, '//li[@id="tab-key-17" and @class="lm-TabBar-tab p-TabBar-tab"]'
).click() # click `From Examples` for input structure
By.XPATH, '//li[@id="tab-key-17"]'
).click() # click `From Examples` tab for input structure
Copy link
Member

Choose a reason for hiding this comment

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

For next time I would recommend to explicitly revert f2d840d and then apply the additional change. You could still do this here and then either rebase and merge or squash merge.

@unkcpz unkcpz force-pushed the fix/205/downstream-ci branch from 54db1c1 to 6ff2acd Compare April 11, 2022 12:06
@unkcpz unkcpz merged commit e96a86f into master Apr 11, 2022
@unkcpz unkcpz deleted the fix/205/downstream-ci branch April 11, 2022 12:09
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.

2 participants