Skip to content

Conversation

@kou
Copy link
Member

@kou kou commented Apr 12, 2024

Rationale for this change

#37821 changed to use add_test() usage from old style to new style:

1a1d2c8?diff=unified&w=1#diff-1ce47eec54afaee769086e1a720c5ed65bc347cd8fc60a233de67fd895dda329L763-R764

MSVC generators multi-config generators. With old style, all tests are run without specifying --build-config explicitly. With new style, we need to specify --build-config explicitly.

See also: https://cmake.org/cmake/help/latest/command/add_test.html

What changes are included in this PR?

Specify --build-config explicitly.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@kou kou requested review from assignUser and raulcd as code owners April 12, 2024 22:01
@kou
Copy link
Member Author

kou commented Apr 12, 2024

@github-actions crossbow submit verify-rc-source-windows

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Apr 12, 2024
@github-actions
Copy link

Revision: 38285b8

Submitted crossbow builds: ursacomputing/crossbow @ actions-3dc9094bdd

Task Status
verify-rc-source-windows GitHub Actions

@kou
Copy link
Member Author

kou commented Apr 13, 2024

+1

@kou kou merged commit 5bdc541 into apache:main Apr 13, 2024
@kou kou deleted the ci-verify-rc-windows branch April 13, 2024 20:19
@kou kou removed the awaiting committer review Awaiting committer review label Apr 13, 2024
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 5bdc541.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

raulcd pushed a commit that referenced this pull request Apr 15, 2024
…41178)

### Rationale for this change

#37821 changed to use `add_test()` usage from old style to new style:

1a1d2c8?diff=unified&w=1#diff-1ce47eec54afaee769086e1a720c5ed65bc347cd8fc60a233de67fd895dda329L763-R764

MSVC generators multi-config generators. With old style, all tests are run without specifying `--build-config` explicitly. With new style, we need to specify `--build-config` explicitly.

See also: https://cmake.org/cmake/help/latest/command/add_test.html

### What changes are included in this PR?

Specify `--build-config` explicitly.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: #41169

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
…ndows (apache#41178)

### Rationale for this change

apache#37821 changed to use `add_test()` usage from old style to new style:

apache@1a1d2c8?diff=unified&w=1#diff-1ce47eec54afaee769086e1a720c5ed65bc347cd8fc60a233de67fd895dda329L763-R764

MSVC generators multi-config generators. With old style, all tests are run without specifying `--build-config` explicitly. With new style, we need to specify `--build-config` explicitly.

See also: https://cmake.org/cmake/help/latest/command/add_test.html

### What changes are included in this PR?

Specify `--build-config` explicitly.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* GitHub Issue: apache#41169

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
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.

1 participant