Skip to content

Fixes to the dist builds for 5.0 - #12292

Merged
Cadair merged 2 commits into
astropy:mainfrom
Cadair:dist_5.0
Oct 27, 2021
Merged

Fixes to the dist builds for 5.0#12292
Cadair merged 2 commits into
astropy:mainfrom
Cadair:dist_5.0

Conversation

@Cadair

@Cadair Cadair commented Oct 26, 2021 •
edited by pllim
Loading

Copy link
Copy Markdown
Member

@github-actions

github-actions Bot commented Oct 26, 2021

Copy link
Copy Markdown
Contributor

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@Cadair
Cadair force-pushed the dist_5.0 branch 2 times, most recently from cb43c73 to ff56016 Compare October 26, 2021 10:55
This was referenced Oct 26, 2021
Closed
Closed
@pllim

pllim commented Oct 26, 2021

Copy link
Copy Markdown
Member

p.s. Since Azure doesn't understand CI skip, I think for now, you can just [ci skip] everything so only Azure runs so we don't waste energy on the regular CI. FYI.

@pllim pllim added this to the v5.0 milestone Oct 26, 2021
@Cadair

Cadair commented Oct 26, 2021

Copy link
Copy Markdown
Member Author

I haven't been as i was trying to make skip work. But yes.

@dhomeier

dhomeier commented Oct 26, 2021

Copy link
Copy Markdown
Contributor

Should test_write_extra_SkyCoord_cols perhaps just be marked xfail until the underlying functionality has been fixed? It tests for a workaround for missing functionality (representing more than one SkyCoord column), which itself does not work consistently across platforms, so I don't see much point in the test in its current form.

@Cadair

Cadair commented Oct 26, 2021

Copy link
Copy Markdown
Member Author

@dhomeier That's probably best discussed in #12170, just skipping it here is fine for now.

@Cadair

Cadair commented Oct 26, 2021

Copy link
Copy Markdown
Member Author

eh mah gawd

image

@Cadair
Cadair marked this pull request as ready for review October 26, 2021 16:01
@Cadair
Cadair requested a review from astrofrog October 26, 2021 16:03
@Cadair Cadair mentioned this pull request Oct 26, 2021
Closed
pllim
pllim reviewed Oct 26, 2021
View reviewed changes
Comment thread azure-pipelines.yml
# Only run one job on PRs
- wheels_cp39*linux_x86_64
- ${{ if ne(variables['Build.Reason'], 'PullRequest') }}:
- sdist

@pllim pllim Oct 26, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought sdist was the one we wanted on PR?

@astrofrog astrofrog Oct 26, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If sdist is added it might still make sense or have one wheel build at least since they use different infrastructure behind the scenes.

@pllim pllim Oct 26, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, which one was broken and we didn't catch? That is the one we need to test per PR.

@Cadair Cadair Oct 27, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They were all broken, the test setup is pretty similar between all of them (clean env, no checkout, dist installed, pytest --pyargs). I picked this as it was a) the fastest and b) also tests the cibuildwheel setup.

@pllim pllim mentioned this pull request Oct 26, 2021
Closed
@Cadair

Cadair commented Oct 27, 2021

Copy link
Copy Markdown
Member Author

:shipit:

@Cadair
Cadair merged commit be0edc7 into astropy:main Oct 27, 2021
@Cadair
Cadair deleted the dist_5.0 branch October 27, 2021 09:04
@Cadair

Cadair commented Oct 27, 2021

Copy link
Copy Markdown
Member Author

I have kicked off a build that will upload a nightly when it's done (which hasn't happened in a long time): https://dev.azure.com/astropy-project/astropy/_build/results?buildId=2136&view=results

@pllim

pllim commented Oct 27, 2021

Copy link
Copy Markdown
Member

Re: nightly -- You never documented how to actually use it...

@Cadair

Cadair commented Oct 27, 2021

Copy link
Copy Markdown
Member Author

Did too!

https://docs.astropy.org/en/latest/install.html?highlight=nightly#installing-pre-built-development-versions-of-astropy

@pllim

pllim commented Oct 27, 2021

Copy link
Copy Markdown
Member

Ops! Sorry for the noise. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim left review comments
@astrofrog astrofrog astrofrog approved these changes

Assignees

No one assigned

Labels

Bug 🔥 Critical no-changelog-entry-needed testing

Projects

None yet

Milestone

v5.0

Development

Successfully merging this pull request may close these issues.

TST: CI should run one Azure Pipelines sdist build in PR sdist & wheel builds broken

5 participants

@Cadair @pllim @dhomeier @astrofrog @odidev