[Github Action] Test and Bench via github actions #63
Conversation
whoops, i meant to open this as a PR for my own fork. since it's here, feel free to look over it - its a moderately faithful recreation of the makefile matrixed across 3.{5,6,7} and mac/linux. so that's neat! |
you can see an example in my testing PRs https://github.com/packysauce/hyperjson/pull/3/checks |
That looks great! Added some notes, but basically we could merge it already. |
Looks great from my side. Do you want me to merge this or are you planning to do any final changes? 😊 |
pipenv is now unhappy about maturin version 0.7.5 not existing. i wanna get
that untangled then I'd be comfortable with a merge. I think it's just the
lockfile
…On Wed, Sep 25, 2019, 2:03 PM Matthias Endler ***@***.***> wrote:
Looks great from my side. Do you want me to merge this or are you planning
to do any final changes? 😊
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#63?email_source=notifications&email_token=AAJXQUM4GXKN3YKX4BXBE3LQLPGYRA5CNFSM4IY2DEE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7TM62I#issuecomment-535220073>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJXQUN2SI2RICU3Z4GQAQDQLPGYRANCNFSM4IY2DEEQ>
.
|
https://github.com/packysauce/hyperjson/pull/5/checks shows all green. I've got py3.6 and py3.7, on the big 3. py3.5 is doable, I think I just need to dust off my "exclude windows" code for it (the right visual studio is not installed by default). For this PR, I may finally rest - feel free to merge at your leisure |
...AGH! I got our forks out of sync! that's frustrating, I'll clean that up
in a bit
…On Fri, Sep 27, 2019, 4:28 PM Matthias Endler ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In Cargo.toml
<#63 (comment)>:
> @@ -1,39 +1,16 @@
[package]
name = "hyperjson"
-version = "0.2.2"
+version = "0.2.1"
We're at 0.2.2 and I kinda got used to the extra metadata fields in the
Cargo.toml. 😉 Do you think we should roll back the changes in this file?
------------------------------
In Makefile
<#63 (comment)>:
> @@ -48,7 +48,7 @@ bench: ## Run benchmarks
pipenv run pytest benchmarks
.PHONY: bench-compare
-bench-compare: nightly dev-packages install ## Run benchmarks and compare results with other JSON encoders
+bench-compare: ## Run benchmarks and compare results with other JSON encoders
Would it hurt to add the dependencies nightly dev-packages install back?
This way, we could also run bench-compare locally. If it interferes with
the pipeline in an unpleasant way I'd be fine either way.
------------------------------
In benchmarks/benchmark_ujson.py
<#63 (comment)>:
> @@ -26,7 +26,6 @@
import ujson
import simplejson
import yajl
- import orjson
No more orjson?
------------------------------
In profiling/Cargo.toml
<#63 (comment)>:
> @@ -2,14 +2,12 @@
name = "profiling"
version = "0.1.0"
authors = ["Matthias Endler ***@***.***>"]
-edition = "2018"
Not sure if that removal was on purpose.
------------------------------
In profiling/src/main.rs
<#63 (comment)>:
> @@ -14,11 +14,16 @@
//! callgrind_annotate --auto=yes callgrind.out.35583 >out.rs
//! qcachegrind callgrind.out.35583
//! ```
+extern crate structopt;
Same for that addition of extern-crate. Am I right that the changes in
this file were added by accident? If not, maybe another git pull origin
master might help.
------------------------------
In src/lib.rs
<#63 (comment)>:
> @@ -1,16 +1,22 @@
#![feature(test)]
+#[macro_use]
Guess we should be good to remove those extern-crates as it's covered by
the compiler.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#63?email_source=notifications&email_token=AAJXQUMDYHMNEKPSDPYJYKDQL2JK7A5CNFSM4IY2DEE2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCGHUDYI#pullrequestreview-294601185>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJXQULZDO7G4PCEJZ4FCBLQL2JK7ANCNFSM4IY2DEEQ>
.
|
Congrats for mastering Github Actions. Just spotted a few minor extraneous changes in the PR, which might be the result of a merge or so. Apart from that we're good to merge. |
43 commits later, I have a working 'test' action
9fdaffa
to
32f0dad
Compare
commits rebased, masters force-pushed, worlds altered. At long last I believe it is ready |
Wow! Great effort. |
This sets up a github action to build/test/bench hyperjson across 3.5, 3.6, 3.7 x macos/ubuntu.
This is my first github action, so things might not be as optimal as they could be.
doesnt help with #60 atm, but deploying wheels looks pretty straightforward now that I'm comfy with actions.