Skip to content

Conversation

@andyundso
Copy link
Member

As I parallelized the cross-compilation process, the gems were moved into another folder dedicated to their target platform during the build. This change was not reflected when storing the artifacts.

As I parallelized the cross-compilation process, the gems were moved into another folder dedicated to their target platform during the build. This change was not reflected when storing the artifacts.
@andyundso andyundso requested a review from aharpervc September 28, 2023 15:11
@aharpervc
Copy link
Contributor

I don't see anything on the artifacts tab here, should that be showing something now? https://app.circleci.com/pipelines/github/andyundso/tiny_tds/2/workflows/e920c589-ee26-4afd-ba5c-fd10bd257931/jobs/35/artifacts

@aharpervc
Copy link
Contributor

I see. That sort of makes sense, although I'm concerned about accessibility. How I checked was I scrolled to the bottom of this PR, clicked "show all checks", clicked into the details for "windows-2.4", clicked artifacts, and didn't see anything.

I think it'd be great if repo visitors had quick access to the fat gems for whichever version of ruby they're using, quickly from each PR.

Thoughts?

@andyundso
Copy link
Member Author

generally agree with you.

I would keep the artifacts upload on the precompile step, as the x86 version is not used in any test. But additionally, upload the artifact from each test, so people can get the correct version for their system, as suggest by you.

will take care of this tomorrow and amend the PR.

@andyundso
Copy link
Member Author

@aharpervc took a while longer, had couple of other things to do first.

but as discussed, now each test on windows uploads the version it has tested as an artifact. would you mind to re-review?

Copy link
Contributor

@aharpervc aharpervc left a comment

Choose a reason for hiding this comment

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

Sweet, LGTM

@andyundso andyundso merged commit 731a43c into rails-sqlserver:master Oct 5, 2023
@andyundso andyundso deleted the fix-artifacts branch October 5, 2023 07:08
@ecentell-CPF ecentell-CPF mentioned this pull request Oct 13, 2023
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