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

Pole 0.5 #25139

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Pole 0.5 #25139

wants to merge 6 commits into from

Conversation

luc-c
Copy link
Contributor

@luc-c luc-c commented Sep 4, 2024

Summary

Changes to recipe: pole/0.5

Motivation

pole is a small library that read and write ole files

Details

Introduction of the library


@CLAassistant
Copy link

CLAassistant commented Sep 4, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Some comments, thanks!

Comment on lines +11 to +15
license = "<Put the package license here>"
author = "<Put your name here> <And your email here>"
url = "<Package recipe repository url here, for issues about the package>"
description = "<Description of pole package here>"
topics = ("<Put some tag here>", "<here>", "<and here>")
Copy link
Member

Choose a reason for hiding this comment

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

Thous would need to be filled

Comment on lines +30 to +31
def export_sources(self):
export_conandata_patches(self)
Copy link
Member

Choose a reason for hiding this comment

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

No need for this if there are no patches :)

tc.generate()

def build(self):
apply_conandata_patches(self)
Copy link
Member

Choose a reason for hiding this comment

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

Same here, no patches, no need for this

Comment on lines +41 to +42
deps = CMakeDeps(self)
deps.generate()
Copy link
Member

Choose a reason for hiding this comment

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

No need for this generator if there are no dependencies


def package(self):
cmake = CMake(self)
cmake.install()
Copy link
Member

Choose a reason for hiding this comment

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

We'll also need to package the license file to a license folder in this method, check how this is done in the cmake_package template in https://github.com/conan-io/conan-center-index/tree/master/docs/package_templates/cmake_package


# Optional metadata
license = "<Put the package license here>"
author = "<Put your name here> <And your email here>"
Copy link
Member

Choose a reason for hiding this comment

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

Follow https://github.com/conan-io/conan-center-index/tree/master/docs/package_templates/cmake_package for what values to actually put here (Or use that template directly!), but author is not a field that should be added to a CCI recipe :)

Suggested change
author = "<Put your name here> <And your email here>"

@@ -0,0 +1,4 @@
sources:
"0.5":
url: "https://github.com/luc-c/Pole/archive/refs/tags/0.5.zip"
Copy link
Member

Choose a reason for hiding this comment

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

I know this is your fork, but I'd like to ask what's the difference with the original https://github.com/otofoto/Pole and why one would be chosen above there other, thanks!

Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Didn't meant to approve!

@luc-c
Copy link
Contributor Author

luc-c commented Sep 5, 2024

Some comments, thanks!

Thanks for your feedback!
I'll certainly change my strategy and make a patch on top of the "original" fork!

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.

4 participants