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.

Sign up for GitHub

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

Jump to bottom

[sdformat6] Migrate from Bitbucket to GitHub 🤖 #10859

Merged
merged 3 commits into from
Apr 21, 2020
Merged

[sdformat6] Migrate from Bitbucket to GitHub 🤖 #10859

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e14a64e
[sdformat6] Migrate from Bitbucket to GitHub
traversaro Apr 16, 2020
c6ad5af
Cleanup of sdformat6 port
traversaro Apr 17, 2020
6b02eaa
Commit to fixup
traversaro Apr 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ports/sdformat6/CONTROL
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Source: sdformat6
Version: 6.2.0
Version: 6.2.0-1
Homepage: http://sdformat.org/
Build-Depends: boost-any, boost-variant, ignition-math4, urdfdom, tinyxml
Description: Simulation Description Format (SDF) parser and description files.
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that sdformat6 seemed to not support arm and uwp.
So could you please add Supports: !(arm|uwp) field to CONTROL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in c6ad5af .

4 changes: 2 additions & 2 deletions ports/sdformat6/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
include(vcpkg_common_functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

include(vcpkg_common_functions) is no long needed.
Could you please remove this line?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also add vcpkg_fail_port_install(ON_ARCH "arm" ON_TARGET "uwp") here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, no problem. However typically it is common practice in open source project to prefer self-contained PRs that just fix a single problem, and don't mix solving the issue with other (even if required/important) changes. If vcpkg instead suggest to do all the deprecation-related cleanup whenever a port is modified in a PR, that is perfect ok but I suggest to describe this policy in the Mantainer Guideline: https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/maintainer-guide.md#avoid-trivial-changes-in-untouched-files .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in c6ad5af .


vcpkg_from_bitbucket(
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO osrf/sdformat
REF sdformat6_6.2.0
SHA512 3e3934010438bffbf10c1df29bd486c098e3c1bdf2b0349b69a53fb6f4d2bd3b3c8c4b4a8dfb413da13a638c0794f41c1bff4adb11a889b1552d90ba8b94c495
SHA512 3d139ec4b4c9fbfd547ed8bfca0adb5cdca92c1b7cc4d4b554a7c51ccf755b9079c26a006ebfedc5bc5b1ba5e16ad950bb38c47ea97bf97e59a2fd7d12d60620
HEAD_REF sdf6
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please update the way to handle to copyright as
file(INSTALL ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright)?

Copy link
Contributor

Choose a reason for hiding this comment

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

vcpkg_test_cmake() is deprecated.
Could you please remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in c6ad5af .

Expand Down