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

[kf5iconthemes] new port #16567

Merged
merged 37 commits into from
Sep 14, 2021
Merged

[kf5iconthemes] new port #16567

merged 37 commits into from
Sep 14, 2021

Conversation

wrobelda
Copy link
Contributor

@wrobelda wrobelda commented Mar 6, 2021

Describe the pull request

Adds kf5iconthemes. Depends on the following:

@wrobelda wrobelda marked this pull request as draft March 6, 2021 01:24
@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Mar 8, 2021
@NancyLi1013
Copy link
Contributor

Hi @wrobelda

Thanks for adding this new port.

I think it's unnecessary to add too many changes in this PR. You can only do something related to kf5iconthemes. After the dependent PR #14367 merged, you can update tiny changes.

@wrobelda
Copy link
Contributor Author

wrobelda commented Mar 8, 2021

Hi @wrobelda

Thanks for adding this new port.

I think it's unnecessary to add too many changes in this PR. You can only do something related to kf5iconthemes. After the dependent PR #14367 merged, you can update tiny changes.

Yeah, that's the plan. Once #13467 gets merged (which I understand is the one you refer to), I'll rebase this branch against master and it will only have changes related to kf5iconthemes. I opened this up early to see if everything builds fine.

ports/kf5iconthemes/vcpkg.json Outdated Show resolved Hide resolved
versions/k-/kf5iconthemes.json Outdated Show resolved Hide resolved
ports/kf5iconthemes/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

@wrobelda
Thanks for your instant response. I posted some suggestions here, mainly including version updates in json files. Since this is a new policy, it might be not easy to notice this point.

Another question is about the port version. I noticed that there was another new release for these ports. Is it because all ports used version 5.75.0? So you update all ports to the same version?

@NancyLi1013
Copy link
Contributor

NancyLi1013 commented Apr 16, 2021

Wow, Seems you need to rebase this PR now, many changes were brought in this PR. Or you can open a new pull request to adding this port.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout dd3d6df5001d49f954bc39b73a4c49ae3c9e8d15 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 0121857..477c311 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2932,6 +2932,10 @@
       "baseline": "5.84.0",
       "port-version": 1
     },
+    "kf5iconthemes": {
+      "baseline": "5.84.0",
+      "port-version": 0
+    },
     "kf5itemmodels": {
       "baseline": "5.84.0",
       "port-version": 1

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for kf5iconthemes but no changes to version or port version.
-- Version: 5.84.0
-- Old SHA: 5fd63e3d0747a6c3ba2e2f277acbd2e4129b7762
-- New SHA: a34b5beea612f36368ba352aab740da326efb961
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@wrobelda
Copy link
Contributor Author

@NancyLi1013 Windows CI tests were re-enabled and I triggered a run by rebasing, so this is now all ready for a review.

@BillyONeal BillyONeal merged commit 1a13ae0 into microsoft:master Sep 14, 2021
@BillyONeal
Copy link
Member

Thanks for the new port :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants