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

2 packages from leviroth/ocaml-reddit-api at 0.1.0 #17820

Conversation

leviroth
Copy link
Contributor

This pull-request concerns:
-reddit_api_async.0.1.0: Async connection and utility functions for Reddit's API
-reddit_api_kernel.0.1.0: OCaml types for Reddit's API



🐫 Pull-request generated by opam-publish v2.0.2

@camelus
Copy link
Contributor

camelus commented Dec 13, 2020

Commit: ab18d0e

@leviroth has posted 2 contributions.

☀️ All lint checks passed ab18d0e
  • These packages passed lint tests: reddit_api_async.0.1.0, reddit_api_kernel.0.1.0

☀️ Installability check (+2)
  • new installable packages (2): reddit_api_async.0.1.0 reddit_api_kernel.0.1.0

@leviroth leviroth force-pushed the opam-publish-reddit_api_async-reddit_api_kernel.0.1.0 branch 2 times, most recently from 5deb6eb to 8a51158 Compare December 13, 2020 01:15
@leviroth
Copy link
Contributor Author

I tweaked my package to fix a problem in the first CI run, but it looks like the reddit_api_kernel tarball is cached and preventing the check from succeeding. Should I have done something different? Is there a way of clearing the cache?

@mseri
Copy link
Member

mseri commented Dec 14, 2020

There seem to be a chache issue, however the tarball that I get from github has different hashes from what you have in the opam file: I get 3ff3f4f0a43520f403de50cc3c22a335 as md5 for example, instead of dd5205f33dc82235d06fb374bd111949.

I think if you fix the hashes, the CI will clean the cache and restart

@leviroth leviroth force-pushed the opam-publish-reddit_api_async-reddit_api_kernel.0.1.0 branch from 8a51158 to df0e857 Compare December 14, 2020 23:44
@leviroth
Copy link
Contributor Author

Interesting. Thanks for that explanation!

@kit-ty-kate
Copy link
Member

Hi, there was several issues I just fixed in your packages:

  • dune >= 2.0 was listed as required but according to your dune-project file, dune 2.5 is required instead
  • dune should not be tagged as a build dependency (see [RFC] relax parsing of dune-package files dune#2147)
  • your janestreet dependencies were too strict = should almost never be used in opam-repository as it forbids to use bug fix versions and locks users' switches into buggy libraries. >= should almost always be used instead.

Do my changes look correct to you?

@leviroth
Copy link
Contributor Author

Thanks, that all makes sense. Is reddit_api_async.0.1.0's strict dependency on reddit_api_kernel.0.1.0 an issue? In practice I'm not going to bump one without providing a new version of the other, since they're both part of the same project, but I could just as easily relax the dependency if it's better.

@kit-ty-kate
Copy link
Member

Thanks, that all makes sense. Is reddit_api_async.0.1.0's strict dependency on reddit_api_kernel.0.1.0 an issue?

When it's in the same project that's totally fine. Depending on how you plan to make releases it is even recommended in the case where you're planning to release both at the same time (default behaviour if you're using opam-publish or dune-release)

I'm seeing one more issue in the tests on 32bit platforms:

#=== ERROR while compiling reddit_api_async.0.1.0 =============================#
# context              2.1.0~beta4 | linux/x86_32 | ocaml-base-compiler.4.11.1 | pinned(https://github.com/leviroth/ocaml-reddit-api/archive/0.1.tar.gz)
# path                 ~/.opam/4.11/.opam-switch/build/reddit_api_async.0.1.0
# command              ~/.opam/4.11/bin/dune build -p reddit_api_async -j 47 @install @runtest
# exit-code            1
# env-file             ~/.opam/log/reddit_api_async-8-990edf.env
# output-file          ~/.opam/log/reddit_api_async-8-990edf.out
### output ###
# File "test/test_subreddits.ml", line 1, characters 0-0:
#          git (internal) (exit 1)
# (cd _build/default && /usr/bin/git diff --no-index --color=always -u test/test_subreddits.ml test/test_subreddits.ml.corrected)
# diff --git a/test/test_subreddits.ml b/test/test_subreddits.ml.corrected
# index 48a48c6..557766b 100644
# --- a/test/test_subreddits.ml
# +++ b/test/test_subreddits.ml.corrected
# @@ -403,26 +403,26 @@ let%expect_test "subreddit_traffic" =
#        print_s [%sexp (by_date : Subreddit_traffic.By_date.t list)];
#        [%expect
#          {|
# -          (((date 2020-10-24) (uniques 0) (pageviews 0) (subscriptions 0))
# -           ((date 2020-10-23) (uniques 0) (pageviews 0) (subscriptions 0))
# -           ((date 2020-10-22) (uniques 0) (pageviews 0) (subscriptions 0))
# -           ((date 2020-10-21) (uniques 1) (pageviews 7) (subscriptions 0))) |}];
# +          (((date 1952-10-05) (uniques 0) (pageviews 0) (subscriptions 0))
# +           ((date 1952-10-04) (uniques 0) (pageviews 0) (subscriptions 0))
# +           ((date 1952-10-03) (uniques 0) (pageviews 0) (subscriptions 0))
# +           ((date 1952-10-02) (uniques 1) (pageviews 7) (subscriptions 0))) |}];
#        let by_month = List.take (Subreddit_traffic.by_month traffic) 4 in
#        print_s [%sexp (by_month : Subreddit_traffic.By_month.t list)];
#        [%expect
#          {|
# -          (((year 2020) (month Oct) (uniques 3) (pageviews 215))
# -           ((year 2020) (month Sep) (uniques 3) (pageviews 114))
# -           ((year 2020) (month Aug) (uniques 2) (pageviews 58))
# -           ((year 2020) (month Jul) (uniques 5) (pageviews 154))) |}];
# +          (((year 1952) (month Sep) (uniques 3) (pageviews 215))
# +           ((year 1952) (month Aug) (uniques 3) (pageviews 114))
# +           ((year 1952) (month Jul) (uniques 2) (pageviews 58))
# +           ((year 1952) (month Jun) (uniques 5) (pageviews 154))) |}];
#        let by_hour = List.take (Subreddit_traffic.by_hour traffic) 4 in
#        print_s [%sexp (by_hour : Subreddit_traffic.By_hour.t list)];
#        [%expect
#          {|
# -          (((hour "2020-10-24 17:00:00Z") (uniques 0) (pageviews 0))
# -           ((hour "2020-10-24 16:00:00Z") (uniques 0) (pageviews 0))
# -           ((hour "2020-10-24 15:00:00Z") (uniques 0) (pageviews 0))
# -           ((hour "2020-10-24 14:00:00Z") (uniques 0) (pageviews 0))) |}];
# +          (((hour "1952-10-06 13:45:52Z") (uniques 0) (pageviews 0))
# +           ((hour "1952-10-06 12:45:52Z") (uniques 0) (pageviews 0))
# +           ((hour "1952-10-06 11:45:52Z") (uniques 0) (pageviews 0))
# +           ((hour "1952-10-06 10:45:52Z") (uniques 0) (pageviews 0))) |}];
#        return ())
#  ;;
#  

@leviroth
Copy link
Contributor Author

Thanks - I've managed to set up a 32 bit testing environment locally, so hopefully the new version I've pushed will do it!

@kit-ty-kate
Copy link
Member

Thanks a lot!

@kit-ty-kate kit-ty-kate merged commit bf36d41 into ocaml:master Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants