Closed dvirtz closed 5 years ago
Had to change AppVeyor image from 2019 Preview to 2019 as vswhere couldn't find VS installation on the preview images
Hi @dvirtz, thanks for the PR!
I'm afraid that I have never used Conan, so I'll trust you that this all works as it should :-)
My only reservation is regarding versioning: so far I have resisted making any API/ABI promises and having proper releases of NanoRange, because the ranges proposals are still a moving target and (until C++20 is actually finalised) might still change in backwards-incompatible ways.
I see that this PR adds VERSION 0.1.0 to the root CMakeLists.txt. What are the implications of this as far as Conan is concerned? Does it mean that I would be committed to (semver-compliant?) releases in future?
(I note that vcpkg avoids this by only allowing the user to use nanorange with the --head flag, but I don't know if Conan has anything similar.)
I can set the version to be the current git revision. Would that be ok?
I can set the version to be the current git revision. Would that be ok?
This sounds like a good way to go
Good catch, thanks!
On Tue, 29 Oct 2019 at 14:22, Antony Peacock notifications@github.com wrote:
@Twon commented on this pull request.
In conanfile.py https://github.com/tcbrindle/NanoRange/pull/78#discussion_r340042273:
@@ -0,0 +1,26 @@ +from conans import ConanFile, CMake, tools +import ptvsd + +class NanorangeConan(ConanFile):
- name = "nanorange"
- version = "0.1.0"
- license = "BSL-1.0"
- author = "Tristan Brindle (tcbrindle at gmail dot com)"
- url = "https://github.com/tcbrindle/NanoRange"
- description = "Range-based goodness for C++17"
- exports_sources = ("CMakeLists.txt", "single_include/*", "cmake/nanorangeConfig.cmake.in", "LICENSE_1_0.txt")
- generators = "cmake"
- def package(self):
- ptvsd.break_into_debugger()
Do you intend to check in this line to break into the debugger? Look like this might be here while you were debugging the packaging step of the conan configuration.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tcbrindle/NanoRange/pull/78?email_source=notifications&email_token=AB2ORDKM4M2ZKPWYJMAYMHLQRATKBA5CNFSM4JCUEUJ2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJRNFOI#pullrequestreview-308466361, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2ORDPKBIOAFOGAUGCJG6TQRATKBANCNFSM4JCUEUJQ .
Thanks for the review @Twon!
@tcbrindle You're welcome. I think you might need to approve the pull request for it to be merged (don't think I have this permission for your repo). Cheers!
@tcbrindle, if you want the packages uploaded to your repo instead of mine, you should open a bintray repo as described in https://docs.conan.io/en/latest/uploading_packages/bintray/uploading_bintray.html. Then you can update CONAN_LOGIN_USERNAME and CONAN_UPLOAD in .conan/build.py and set the password in travis as described in step 5 of https://docs.conan.io/en/latest/integrations/ci/travisci.html
This should definitely be merged, in the meantime I'v gotten NanoRange included in the new experimental conan-center-index which means it is under the conan-io bintray repository. I set the version to be the date I pulled (and since there hasn't been any updates since, that's what it is now) after advice from some of the bincrafters people.
Hi @dvirtz, is this ready to merge?
As I wrote above it's currently uploading to my Bintray repository. I've no problem to keep it that way but you might want to change it. It could be done at a later time, though.
On Tue, Nov 19, 2019, 00:06 Tristan Brindle notifications@github.com wrote:
Hi @dvirtz https://github.com/dvirtz, is this ready to merge?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tcbrindle/NanoRange/pull/78?email_source=notifications&email_token=AB2ORDIQC6N7S2YOFAGBJ5DQUMGVJA5CNFSM4JCUEUJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEMCO3I#issuecomment-555231085, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2ORDKAHK7EVFGMWQ2HXYLQUMGVJANCNFSM4JCUEUJQ .
Other than that, It's ready
As I wrote above it's currently uploading to my Bintray repository. I've no problem to keep it that way but you might want to change it. It could be done at a later time, though.
I'm happy to leave it that way if you are -- I'm afraid that any bug reports regarding Conan will be coming your way anyway ;-)
It currently uploads to my bintray repo. This can be changed before merging.