Introduce a abstraction for scalar values #251

weiznich · 2018-09-14T12:58:31Z

Disclaimer 😉

So first I want to say sorry for opening such a big PR, but I see no way to do this in several smaller steps, because this basically needs to change/touch ~everything in juniper.

My main motivation for doing this is to finally solve the discussion about which scalar type should be supported and which not, because wundergraph will need to have some sort of custom scalar values that are not part of the graphql core standard.

Next thing: This is not finished yet, but I decided to open a PR early to start a discussion about the general idea.
In detail at least the following things needs to be fixed before we could talk about merging this

Fixing all tests
Clean up some parts of the code (somewhere are debug println! calls)
Fixing all the other crates in the repo
Add some documentation
Add handling of the new generic parameter to some macros (They just set it internally currently, but similar to the graphql_scalar! macro there should be a way to specify the type of the used scalar value representation)
Rebase to master

cc #232 and #235 (Basically this would allow implementing all custom integrations outside of juniper itself, if I do not miss something)

Description

Before this possible scalar values where hard coded to be representable
in one of the following types: i32, f64, String or bool. This
restricts what types of custom scalar values could be defined. (For
example it was not possible to define a scalar value that represents a
i64 without mapping it to a string (that would be not efficient))

One solution to fix that example above would simply be to change the
internal representation to allow it represent a i64, but this would
only fix the problem for one type (till someone want's to support
i128 for example…). Also this would make juniper not following the
graphql standard closely.

This commit tries to explore another approach, by making the exact "internal"
representation of scalar values swappable (in such a way that a down
stream crate could provide it's own representation tailored to their
needs). This allows juniper to provide a default type that only
contains the types described in the standard whereas other crates
could define custom scalars following their needs.

To that we need to change several things in the current implementation

Add some traits that abstract the behaviour of such a scalar value representation
Change Value and InputValue to have a scalar variant (with a
generic type) instead of hard coded variants for the standard
types. This implies adding a generic parameter to both enums that
needs to be added in the whole crate.
Change the parser to allow deciding between different types of
scalar values. The problem is basically that the original parser
implementation had no way to know whether a parsed integer number is
a i32 or a i64 for example. To fix this we added some knowlege
from the existing schema to the parser.
Fix some macros and derives to follow the new behaviour

This also contains a unrelated change about the way juniper_codegen
resolves items from juniper. In detail the _internal flag is removed
the the resolution is replaced by an macro. This is a drive by change
because I was annoyed of adding new items to that list.

LegNeato

Thanks for working on this! I took a quick superficial review pass.

My main concern overall is that it makes the public API harder/uglier to use for the simple case. It would be good if we can find a way that projects not needing custom scalars don't have to worry about them in their type signatures and macros. Not sure if that is possible though...

juniper/src/lib.rs

LegNeato · 2018-09-17T18:30:28Z

juniper/src/integrations/uuid.rs

 use Value;

-graphql_scalar!(Uuid {
+graphql_scalar!(Uuid where Scalar = <S> {


I really don't like this public API. Is it possible to default to <S> when there is not a where Scaler = in the macro?

It already defaults to <__S> but in my opinion (at least in this case) it is more readable if there is no type parameter coming from somewhere.

In cases where someone want's to define a new scalar type based on an existing one, the where Scalar = … clause is not necessary, because the from_str function could be written in terms of the existing type (for example <String as ParseScalarValue<_>>::from_str(value)).

For cases where a custom scalar type is used the scalar type needs to be specified explicitly.

Is it possible to default to <S> when there is not a where Scaler = in the macro?

In theory this should be possible, but then there would be a generic parameters coming from somewhere. In my opinion that's noting we should do .
Currently the default is set to DefaultScalarValue. All other variants need some syntax to specify them. If someone has a better idea than where Scalar = …, that should be quite easy to change.)

LegNeato · 2018-09-17T18:32:23Z

juniper/src/macros/object.rs

@@ -137,11 +137,11 @@ automatically via the `?` operator, or you can construct them yourself using
 struct User { id: String }

 graphql_object!(User: () |&self| {
-    field id() -> FieldResult<&String> {
+    field id() -> FieldResult<&String, __S> {


I really don't like this public API. Is it possible to default FieldResult to the proper type in the simple case and introduce something like CustomScalarFieldResult for the custom case?

Similar to the execute function it is possible to define type aliases with default types, so the definition of FieldResult would look like this: type FieldResult<T, S = DefaultScalarValue> = Result<T, FieldError<S>>;

(Sidenote: In this case this would not have been helpful, because the graphql_object! macros introduces internally a generic scalar type (__S). This macro needs to be rewritten in a similar manner like the graphql_scalar! macro)

It should be much more clearer now.
It is either FieldResult<&String> for using the DefaultScalarValue or FieldResult<&String, ScalarType> for using a custom type or a generic parameter. In both cases this type is introduced in the header of the macro.

juniper/src/executor_tests/enums.rs

weiznich

Thanks for your quick review.

My main concern overall is that it makes the public API harder/uglier to use for the simple case. It would be good if we can find a way that projects not needing custom scalars don't have to worry about them in their type signatures and macros. Not sure if that is possible though.

It is totally possible to default to DefaultScalarValue in almost all cases. (See the specific comments I've added for examples). I've not done that yet because it is easier to introduce the generic type everywhere without default values.

That said I think we need to discuss where exactly we want to introduce this default values. Out of my mind there are the following places where it may be possible:

The main entry point function for executing queries (fn execute)
The definiton of the main schema type (RootNode)
The definition of the GraphQLType trait
The macros provided to implement various types of graphql types. (graphql_scalar!, graphql_object!, …)
The definition of the underlying value types (InputValue, Value, LookAheadValue)

At least for the macros I'm not sure if we would like to prefere a general implementation for all scalar types in place of defaulting to implement it only for one (the DefaultScalarValue) type.

Overall I think it may be possible to provide an api that is equal to the current one and only differs in the following places:

Implementation/Definition of a scalar value. This requires now adding the from_str(value: &str) function to the call of graphql_scalar! to implement ParseScalarValue<S>
Direct interaction with Value, InputValue or LookAheadValue. This requires to change the interaction with the variants. For example Value::Int -> Value::Scalar(DefaultScalarValue::Int())

weiznich · 2018-09-17T19:32:30Z

juniper/src/integrations/uuid.rs

 use Value;

-graphql_scalar!(Uuid {
+graphql_scalar!(Uuid where Scalar = <S> {


It already defaults to <__S> but in my opinion (at least in this case) it is more readable if there is no type parameter coming from somewhere.

In cases where someone want's to define a new scalar type based on an existing one, the where Scalar = … clause is not necessary, because the from_str function could be written in terms of the existing type (for example <String as ParseScalarValue<_>>::from_str(value)).

For cases where a custom scalar type is used the scalar type needs to be specified explicitly.

juniper/src/lib.rs

weiznich · 2018-09-17T19:39:31Z

juniper/src/macros/object.rs

@@ -137,11 +137,11 @@ automatically via the `?` operator, or you can construct them yourself using
 struct User { id: String }

 graphql_object!(User: () |&self| {
-    field id() -> FieldResult<&String> {
+    field id() -> FieldResult<&String, __S> {


Similar to the execute function it is possible to define type aliases with default types, so the definition of FieldResult would look like this: type FieldResult<T, S = DefaultScalarValue> = Result<T, FieldError<S>>;

(Sidenote: In this case this would not have been helpful, because the graphql_object! macros introduces internally a generic scalar type (__S). This macro needs to be rewritten in a similar manner like the graphql_scalar! macro)

weiznich · 2018-09-19T21:39:52Z

I've pushed some updates fixing most things that where open in the initial version.

weiznich · 2018-09-20T14:54:50Z

I've pushed another bunch of changes. Now everything should be building, all tests should be passing and there should be no merge conflict.

weiznich · 2018-09-21T21:16:41Z

Remaining CI errors are looking like not to be caused by this change 🙈

The 1.22/1.23 builder fails because some dependency uses recently stabilised
features.
The stable/beta builder fails because it tries to compile juniper_rocket that requires nightly.

LegNeato · 2018-09-24T16:34:15Z

I'll take a look at this later in the week, but CI should pass here...it's currently passing on master.

weiznich · 2018-09-24T19:02:55Z

I'll take a look at this later in the week, but CI should pass here...it's currently passing on master.

I totally agree with you that the ci should pass, but I do not see what needs to be fixed (beside of editing the ci config…)
There are the following errors:

AppVeyor:
- nightly gnu target some git failure, looks like that probably goes away by trying to rebuild.
Travis:
- stable Tries to build juniper-rocket. This will never succeed on stable because of the dependency on rocket
- beta Tries to build juniper-rocket. This will never succeed on beta because of the dependency on rocket
- 1.23 Fails to build because crossbeam (dependency of wrap) seems to use a feature that requires a newer rust version
- 1.22 Fails to build because lazycell (dependency of wrap) seems to use a feature that requires a newer rust version

LegNeato · 2018-09-24T21:58:32Z

Right, looks like something got messed up here in a merge/rebase? Onmaster:

juniper_rocket is only built on nightly.
juniper_warp has a separate line in the travis config and is only run on certain builders.

It looks like this PR branch doesn't behave similarly.

weiznich · 2018-09-25T12:26:50Z

Right, looks like something got messed up here in a merge/rebase?

Whatever happened I've edited the ci config in that way that juniper_rocket is only build on nightly and juniper_wrap is only build on >=stable.
Now the CI passes.

LegNeato

This looks awesome, thanks so much for doing the work and sticking with the change! I just have a couple of questions. I'd also love for @theduke and/or @mhallin to take a look before landing.

We'll also need a blurb added to the changelog before landing as well.

LegNeato · 2018-09-26T00:05:21Z

juniper/src/ast.rs

@@ -496,45 +541,48 @@ impl<'a> VariableDefinitions<'a> {
 mod tests {
    use super::InputValue;
    use parser::Spanning;
+    use value::DefaultScalarValue;
+
+    type TestValue = InputValue<DefaultScalarValue>;


Is this needed anymore now that you have a default type?

This could be replaced by InputValue, but the type annotation will be required in this case. (Because there is no way for rustc to see which type should be used for S here)

LegNeato · 2018-09-26T00:07:59Z

juniper/src/executor/look_ahead.rs

@@ -544,7 +569,7 @@ query Hero {

    #[test]
    fn check_query_with_argument() {
-        let docs = ::parse_document_source(
+        let docs = parse_document_source(


Why does this not need the turbofish and the other calls do?

Because this is the only test that explicitly uses a DefaultScalarValue in it's expected results.
All other tests don't use scalar values, so we must tell rustc which scalar value type should be used.

LegNeato · 2018-09-26T00:24:49Z

juniper/src/macros/common.rs

+    (
+        callback = $callback:ident,
+        rest = <$($lifetime:tt),*> $name: ty $(: $ctxt: ty)* as $outname: tt
+        where Scalar = <$generic:tt $(: $bound:tt)*> $(| &$mainself:ident |)* {


I'd still like to make the where Scalar clause optional in macros and default insert a where Scalar = <S> when omitted. This will ease upgrade pain and means every graphql_object! call doesn't need to be updated, only those with custom scalars.

There is already a default case without where Scalar clause. It defaults to using DefaultScalarValue because otherwise FieldResult and such things wouldn't work. (If we would default to a generic type S each field returning a result could not just return FieldResult<T> as in juniper <=0.10, but needs to return FieldResult<T, S> with some magic type S.)

Oh, I missed that....the macros are huge 😄. Let's change the integrations to omit the clause then (example: https://github.com/graphql-rust/juniper/pull/251/files/aa357f7935d0a361f17ded2ae5c69d64839f7c3e#diff-e1dde797a5a7df30690ff359d95079d0R25)

LegNeato · 2018-09-26T00:28:38Z

juniper/src/integrations/url.rs


    #[test]
    fn url_from_input_value() {
        let raw = "https://example.net/";
-        let input = ::InputValue::String(raw.to_string());
+        let input: ::InputValue<DefaultScalarValue> = ::InputValue::scalar(raw.to_string());


Is InputValue<DefaultScalarValue> needed here or does type inference handle it?

: InputValue is needed, type inference could not handle this otherwise.

weiznich · 2018-09-26T21:54:30Z

If we change the integrations to use the default clause only `DefaultScalarValue` will be supported. In my opinion that would be quite surprising. The current implementation applies to all scalar value representations out there. So not everybody needs to write the same interactions again and again. Am 26. September 2018 23:12:12 MESZ schrieb Christian Legnitto <[email protected]>:

…

LegNeato commented on this pull request. > + }; + ( + $scalar: ty + ) => { + $scalar + }; +} + + +#[doc(hidden)] +#[macro_export] +macro_rules! __juniper_parse_object_header { + ( + callback = $callback:ident, + rest = <$($lifetime:tt),*> $name: ty $(: $ctxt: ty)* as $outname: tt + where Scalar = <$generic:tt $(: $bound:tt)*> $(| &$mainself:ident |)* { Oh, I missed that....the macros are huge 😄. Let's change the integrations to omit the clause then (example: https://github.com/graphql-rust/juniper/pull/251/files/aa357f7935d0a361f17ded2ae5c69d64839f7c3e#diff-e1dde797a5a7df30690ff359d95079d0R25) -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #251 (comment)

LegNeato · 2018-09-26T22:11:26Z

Sorry, I'm not being clear. I'll put up a patch showing what I mean.

Edit: Ok, I see what you are saying above now that I worked on a patch :-D.

weiznich · 2018-09-28T11:34:22Z

AppVeyor build failed with an unrelated network error.
@theduke @mhallin Would be great to get a review on this.

Before this possible scalar values where hard coded to be representable in one of the following types: `i32`, `f64`, `String` or `bool`. This restricts what types of custom scalar values could be defined. (For example it was not possible to define a scalar value that represents a `i64` without mapping it to a string (that would be not efficient)) One solution to fix that example above would simply be to change the internal representation to allow it represent a `i64`, but this would only fix the problem for one type (till someone want's to support `i128` for example…). Also this would make juniper not following the graphql standard closely. This commit tries to explore another approach, by making the exact "internal" representation of scalar values swappable (in such a way that a down stream crate could provide it's own representation tailored to their needs). This allows juniper to provide a default type that only contains the types described in the standard whereas other crates could define custom scalars following their needs. To that we need to change several things in the current implementation * Add some traits that abstract the behaviour of such a scalar value representation * Change `Value` and `InputValue` to have a scalar variant (with a generic type) instead of hard coded variants for the standard types. This implies adding a generic parameter to both enums that needs to be added in the whole crate. * Change the parser to allow deciding between different types of scalar values. The problem is basically that the original parser implementation had no way to know whether a parsed integer number is a `i32` or a `i64` for example. To fix this we added some knowlege from the existing schema to the parser. * Fix some macros and derives to follow the new behaviour This also contains a unrelated change about the way `juniper_codegen` resolves items from juniper. In detail the `_internal` flag is removed the the resolution is replaced by an macro. This is a drive by change because I was annoyed of adding new items to that list.

This reworks the parser again, because the previous approach was to strict. Now the strategy is as following: * Pass optional type information all the way down in the parser. If a field/type/… does note exist, just do not pass down the type information * The lexer does now again distinguish between several fundamental scalar types (String, Float, Int). This is done on a fundamental level. It does not try to actually parse those values, just annotate them that this is a floating point number, a integer number or a string value * If those type information exist while parsing a scalar value, try the following: 1. Try parsing the value using that type information 2. If that fails try parsing the value using the inferred type information from the lexer * If no type information exist try parsing the scalar value using the inferred type from the lexer

…type To do this some refactoring/moving of some internal transformations are required The only thing left for manual implementation is `Deserialize` because I see no way to infer that just from the enum.

Those methods are related to scalar values, end users should better use the generic variant of those methods.

This commit changes all provided macros to support the introduced scalar value abstraction. It is now possible to specify if a certain implementation should be based on a specific scalar value representation or be generic about the exact representation. All macros now defaulting to the `DefaultScalarValue` type provided by juniper if no scalar value representation is specified. This is done with usability and backwards compatibility in mind. To implement this feature more or less the whole macro code is rewritten. Now there is a clear separation between parsing the custom dsl and expanding the final impls. Also the new code tries to share much of the implementation between the different macros.

In general this change should allow old to work without many explicit types everywhere. This also removes some unused trait bounds.

* There was an issue while serializing values from json * use `graphql_object!` for implementing `GraphQLType`

Allow to specify the used scalar value representations via a attribute (`#[graphql(scalar = "Type")]`). As default a generic implementation is provided because that will work in most cases.

* Fix issues with the GraphQLObject derive * Move custom_scalar test here because this is something that should be possible outside of juniper. In that way we ensure that this is tested.

This fix introduces a new way to convert a scalar value into one of the 4 standardized scalar values (Int, Float, String, Bool). Using this abstraction it is possible to do certain needed conversations in custom scalar value code, for example that one that converts a integer value to a floating point value.

codecov-io · 2018-10-19T13:12:53Z

Codecov Report

Merging #251 into master will increase coverage by 1.28%.
The diff coverage is 94.61%.

@@            Coverage Diff             @@
##           master     #251      +/-   ##
==========================================
+ Coverage   87.53%   88.82%   +1.28%     
==========================================
  Files          97      102       +5     
  Lines       14200    20718    +6518     
==========================================
+ Hits        12430    18402    +5972     
- Misses       1770     2316     +546

Impacted Files	Coverage Δ
juniper/src/validation/traits.rs	`100% <ø> (ø)`	⬆️
...per/src/validation/rules/unique_operation_names.rs	`96.93% <ø> (+0.44%)`	⬆️
juniper_tests/src/codegen/derive_input_object.rs	`79.03% <ø> (+14.85%)`	⬆️
.../src/validation/rules/variables_are_input_types.rs	`94.82% <ø> (+0.54%)`	⬆️
juniper_codegen/src/derive_input_object.rs	`0% <ø> (ø)`	⬆️
juniper/src/validation/visitor.rs	`98.42% <ø> (+1.04%)`	⬆️
juniper_warp/src/lib.rs	`91.66% <ø> (ø)`
...iper/src/validation/rules/unique_variable_names.rs	`97.4% <ø> (+1.17%)`	⬆️
juniper/src/validation/test_harness.rs	`87.42% <ø> (+2.93%)`	⬆️
juniper_codegen/src/lib.rs	`3.7% <ø> (-2.55%)`	⬇️
... and 168 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd455c9...b9aa072. Read the comment docs.

weiznich · 2018-10-19T13:38:53Z

@LegNeato @theduke @mhallin I've rebased everything and added a changelog entry, so this is ready for a hopefully final review.

LegNeato · 2018-10-23T03:28:49Z

It's Monday...today we land! @theduke and @mhallin can chime in after the merge if they think this isn't the proper direction.

weiznich force-pushed the feature/more_scalar_types branch from 796d295 to 805db63 Compare September 14, 2018 13:03

LegNeato requested review from mhallin and theduke September 17, 2018 18:11

LegNeato requested changes Sep 17, 2018

View reviewed changes

weiznich commented Sep 17, 2018

View reviewed changes

weiznich force-pushed the feature/more_scalar_types branch from 45fcc3e to 5f17d0b Compare September 20, 2018 14:52

weiznich changed the title ~~[WIP] Introduce a abstraction for scalar values~~ Introduce a abstraction for scalar values Sep 20, 2018

weiznich force-pushed the feature/more_scalar_types branch 4 times, most recently from 4b2589b to aa357f7 Compare September 25, 2018 11:58

LegNeato reviewed Sep 26, 2018

View reviewed changes

LegNeato mentioned this pull request Oct 2, 2018

Use latest cargo make #260

Merged

weiznich mentioned this pull request Oct 15, 2018

Support custom scalar types #232

Closed

weiznich added 6 commits October 19, 2018 11:29

Remove the allocation from the try_parse_fn callback

159d866

Add a custom derive to simplify the setup of new custom scalar value …

a382da1

…type To do this some refactoring/moving of some internal transformations are required The only thing left for manual implementation is `Deserialize` because I see no way to infer that just from the enum.

Add some documentation

b7d21f6

Deprecated some methods on InputValue and Value

3def216

Those methods are related to scalar values, end users should better use the generic variant of those methods.

weiznich added 13 commits October 19, 2018 11:30

Add DefaultScalarValue as default type for some generics

bac6948

In general this change should allow old to work without many explicit types everywhere. This also removes some unused trait bounds.

Improve custom scalar type test case

3243116

* There was an issue while serializing values from json * use `graphql_object!` for implementing `GraphQLType`

Extend derives to support custom scalar value representations

955d698

Allow to specify the used scalar value representations via a attribute (`#[graphql(scalar = "Type")]`). As default a generic implementation is provided because that will work in most cases.

Fix juniper_iron

435e85e

Fix juniper_tests

ace8714

* Fix issues with the GraphQLObject derive * Move custom_scalar test here because this is something that should be possible outside of juniper. In that way we ensure that this is tested.

Fix juniper_rocket

69f2a0c

Fix errors indroduced by the rebase

b545273

Fix compile on rust 1.22

a115ca1

Try fixing the ci

996d5d9

Some minor tweaks

0e2156f

Add a changelog entry

16d0286

weiznich force-pushed the feature/more_scalar_types branch from 11021ea to 16d0286 Compare October 19, 2018 11:57

Fix rebase errors

b9aa072

LegNeato merged commit 2e5df9f into graphql-rust:master Oct 23, 2018

theduke mentioned this pull request Dec 18, 2018

Unable to import macros individually [regression] #296

Closed

LegNeato mentioned this pull request Jan 15, 2019

Unions example from book doesn't compile #315

Closed

weiznich deleted the feature/more_scalar_types branch September 18, 2019 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce a abstraction for scalar values #251

Introduce a abstraction for scalar values #251

weiznich commented Sep 14, 2018 •

edited

Loading

LegNeato left a comment

LegNeato Sep 17, 2018

weiznich Sep 17, 2018

weiznich Sep 19, 2018 •

edited

Loading

LegNeato Sep 17, 2018

weiznich Sep 17, 2018

weiznich Sep 19, 2018

weiznich left a comment •

edited

Loading

weiznich Sep 17, 2018

weiznich Sep 17, 2018

weiznich commented Sep 19, 2018

weiznich commented Sep 20, 2018

weiznich commented Sep 21, 2018

LegNeato commented Sep 24, 2018

weiznich commented Sep 24, 2018

LegNeato commented Sep 24, 2018 •

edited

Loading

weiznich commented Sep 25, 2018

LegNeato left a comment

LegNeato Sep 26, 2018

weiznich Sep 26, 2018

LegNeato Sep 26, 2018

weiznich Sep 26, 2018

LegNeato Sep 26, 2018

weiznich Sep 26, 2018

LegNeato Sep 26, 2018

LegNeato Sep 26, 2018

weiznich Sep 26, 2018

weiznich commented Sep 26, 2018 via email

LegNeato commented Sep 26, 2018 •

edited

Loading

weiznich commented Sep 28, 2018

codecov-io commented Oct 19, 2018

weiznich commented Oct 19, 2018

LegNeato commented Oct 23, 2018

Introduce a abstraction for scalar values #251

Introduce a abstraction for scalar values #251

Conversation

weiznich commented Sep 14, 2018 • edited Loading

Disclaimer 😉

Description

LegNeato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiznich Sep 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiznich left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiznich commented Sep 19, 2018

weiznich commented Sep 20, 2018

weiznich commented Sep 21, 2018

LegNeato commented Sep 24, 2018

weiznich commented Sep 24, 2018

LegNeato commented Sep 24, 2018 • edited Loading

weiznich commented Sep 25, 2018

LegNeato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiznich commented Sep 26, 2018 via email

LegNeato commented Sep 26, 2018 • edited Loading

weiznich commented Sep 28, 2018

codecov-io commented Oct 19, 2018

Codecov Report

weiznich commented Oct 19, 2018

LegNeato commented Oct 23, 2018

weiznich commented Sep 14, 2018 •

edited

Loading

weiznich Sep 19, 2018 •

edited

Loading

weiznich left a comment •

edited

Loading

LegNeato commented Sep 24, 2018 •

edited

Loading

LegNeato commented Sep 26, 2018 •

edited

Loading