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

tidyjson recipe file #8619

Closed
wants to merge 8 commits into from
Closed

tidyjson recipe file #8619

wants to merge 8 commits into from

Conversation

sbilge
Copy link

@sbilge sbilge commented Jun 24, 2019

  • License file is packaged (see here for an example)
  • Source is from official source
  • Package does not vend other packages
  • Build number is 0
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/tidyjson) and found it was in an excellent condition.

@sbilge
Copy link
Author

sbilge commented Jun 24, 2019

@conda-forge/core @conda-forge/help-r @conda-forge/staged-recipes Would you please review my code? Tests are failing but I am not sure why.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/r-tidyjson) and found it was in an excellent condition.

@jdblischak
Copy link
Member

@sbilge Thanks for contributing this recipe!

The tests are failing because you have not included the build scripts that accompany the metadata file:

> library('tidyjson')
Error in library("tidyjson") : there is no package called 'tidyjson'
Execution halted

https://ci.appveyor.com/project/conda-forge/staged-recipes/builds/25498077#L485

To make the submission process go as smoothly as possible, could you please use the conda-forge helper script for R recipes? You can read about this and other contributing instructions in the documentation on contributing packages.

A few notes:

  • After you run the helper script, then please add yourself again as a maintainer
  • You don't need to include a license file for the MIT license. The conda-forge R maintainers haven't decided on a consistent method for packaging this file, so for now we just exclude it.

@sbilge
Copy link
Author

sbilge commented Jun 24, 2019

@jdblischak Thank you very much!

@jdblischak
Copy link
Member

@sbilge Please let me know once you've updated the recipe using the helper script. The helper script will make the recipe noarch (because it doesn't have any compiled code) and apply the standard formatting we use for all R recipes (this consistency makes it easier to maintain them all).

@dbast
Copy link
Member

dbast commented Jun 24, 2019

The package was removed from cran. That means using the r_skeleton_helper is maybe not that straight forward.

@jdblischak
Copy link
Member

The package was removed from cran. That means using the r_skeleton_helper is maybe not that straight forward.

@dbast Good point! I'll make my comments line-by-line


source:

url: https://github.com/sbilge/tidyjson/archive/v0.2.2.3.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

This is not great. This is your personal fork of the repository.

What is the status of this package? Is colearendt the new maintainer? While a CRAN release is the best case scenario, at minimum it would be ideal if the maintainer tagged a release on the offical GitHub repository. I see you started the discussion in colearendt/tidyjson#111

Have you confirmed that you need version 0.2.2.3? In other words, would 0.2.2 be sufficent?

Also, have you considered creating a conda package and uploading it to your personal channel on Anaconda Cloud? If this R package is not well-supported, it isn't a good fit for conda-forge (because it will be hard for us to maintain it).

Copy link
Author

Choose a reason for hiding this comment

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

Yes, colearendt is the maintainer. I reached him via his email and requested a GitHub release. He said he will make the release but he did not, yet. I have to continue working on my PhD project, so I decided to use my own fork until his release. 0.2.2 is a very old version (from 2015), it would not be sufficient unfortunately.

Would it be alright if we continue with my fork for now and I update the recipe once colearendt release it? Otherwise I can try Anaconda personal channel.

Copy link
Member

Choose a reason for hiding this comment

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

@sbilge My preference would be for you to create a personal conda package until the maintainer releases a new version (preferably to CRAN, but at minimum a GitHub tag).

@conda-forge/r Any opinions on maintaining this recipe? I know I'm always the curmudgeon about maintaining non-CRAN packages...

Copy link
Member

Choose a reason for hiding this comment

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

Well, the given URL is neither CRAN nor any upstream supported release URL (sometimes forks become the new upstream, but the commit history does not indicate that). So I don't think you are the curmudgeon here ;)

@sbilge you can upload the package to your own anaconda.org channel, the steps are something like:

  • create an anaconda.org account (free)
  • create an upload $token inside that account
  • install conda-build + anaconda-client
  • build+upload via conda-build staged-recipes/recipes/r-tidyjson --token $token
  • Install/use the package via: conda install -c $yourChannel r-tidyjson or conda install $yourChannel:r-tidyjson or add it to your environment.yml.

That should not take that long... Would that be sufficient?

This PR can be updated and merged after upstream did tag a new release.

Copy link
Author

Choose a reason for hiding this comment

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

@dbast @jdblischak Thank you. I will do so. Should I close the pull request till I update the PR with the original release?

Copy link
Member

Choose a reason for hiding this comment

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

@sbilge Yes, please close it for now so that we know that it is not in need of our attention. You can reopen it once it's been updated for the new release. Thanks!

recipes/r-tidyjson/meta.yaml Outdated Show resolved Hide resolved
recipes/r-tidyjson/meta.yaml Show resolved Hide resolved
recipes/r-tidyjson/meta.yaml Show resolved Hide resolved
recipes/r-tidyjson/meta.yaml Show resolved Hide resolved
recipes/r-tidyjson/meta.yaml Outdated Show resolved Hide resolved
recipes/r-tidyjson/meta.yaml Show resolved Hide resolved
recipes/r-tidyjson/build.sh Outdated Show resolved Hide resolved
@sbilge sbilge closed this Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants