update to argonaut 7 and affjax 11 #13

srghma · 2020-11-16T16:53:35Z

No description provided.

cryogenian · 2020-11-16T19:11:22Z

Please don't remove bower for now. Having both spago and bower seems redundant but not a big deal I think 🙂

srghma · 2020-11-17T13:03:42Z

@cryogenian done

srghma · 2020-11-18T17:06:09Z

@cryogenian in 661bb41

BaseEffects or LunaparkBaseEffects (as is) ?

cryogenian

Seems almost done! 👍

Just a couple of smallish comments

src/Lunapark/API.purs

cryogenian · 2020-11-18T18:34:48Z

src/Lunapark/ActionF.purs

@@ -32,49 +33,49 @@ derive instance functorTouchF ∷ Functor TouchF

 _lunaparkActions = SProxy ∷ SProxy "lunaparkActions"
 type LUNAPARK_ACTIONS = R.FProxy ActionF
-type WithAction r = R.Run (lunaparkActions ∷ LUNAPARK_ACTIONS|r) Unit
+type LunaparkActionsEffect r = ( lunaparkActions ∷ LUNAPARK_ACTIONS | r )


I don't mind, but could you please share your motivation here? Is it because BaseEffect andWithAction is unclear?

BaseEffect? You prob mean BaseRun

the reason is that it's easier to understand when Run is not in type

do one thing and do it well

it's better to not use type aliases when possible (I have learned this from decoder implementation that is not dependent on class -> move implementation to Implementation module purescript-contrib/purescript-argonaut-codecs#74 (comment) I think F in Foreign is a similar case, and a mistake also, as again the underlying types are pretty simple without the synonym)

This change means that instead of this

myAwesomeTest :: forall r. Lunapark r myAwesomeTest = do go "http://whatever.com" clickEl selector

End-users will need to write this

myAwesomeTest:: forall r. Run (LunaparkBaseEffects + LunaparkEffect + LunaparkActionsEffect + r) myAwesomeTest = do go "http://whatever.com" clickEl selector

Removing Lunapark r complicates basic use-case and propagates implementation details to the call site, I think it should be preserved. I'm fine with removing WithAction, BaseRun and using Run (Whatever r) inside the library, but don't agree with removing main library monad synonim from public API. I'd say that making Lunapark r a newtype might be good idea, but not in this pr 🙂

Personally, I think that this Lunapark r (not WithAction r and definitely not BaseRun r) are somewhat like CodecJson, not F (ugh... F from foreign is terrible)

returned type Lunapark

re newtyping - not sure if I like this idea

I'm fine with doing

Run (LunaparkBaseEffects + LunaparkEffect + LunaparkActionsEffect + r)

actually I removed BaseRun so I could easiter to this to see what is going on

I like when I see ok, I'm using the Run lib, not something else

src/Lunapark/Endpoint.purs

cryogenian · 2020-11-18T18:39:21Z

src/Lunapark/Error.purs

+  = JsonDecodeError J.JsonDecodeError
+  | WebdriverError LWE.WebdriverError
+  | AffjaxError N.Error
+  | UserError String


This one is actually LunaparkError not UserError as it's internal thing for lunapark

@cryogenian

how about rename UserError to InternalError (since it is only in lib)

and rename LunaparkF to BaseActionF?

I think there is too much Lunapark in lunapark

or maybe LunaparkF to ActionF and ActionF to CompoundActionF?

Well it's not internal, right? More like it's compound or domain level, what about CompoundError?

LunaparkF -> ActionF and ActionF -> CompoundActionF break webdriver naming scheme, because ActionF is actually about actions in webdriver (they are encoded as action field and discussed in action section of docs. I'd agree that LunaparkActionEffect -> ActionEffect is OK, but, yeah, ActionEffect sounds a bit too generic to me, although it's definitely acceptable.

re errors

I think I have found the best variant e7428d9

re renaming

I have finally understood the ActionF

it transforms

my = do ActionF.sendKeys "a" ActionF.pause LunaparkF.foo ActionF.sendKeys "a" ActionF.pause

to

my = do LunaparkF.performActions $ Object.fromHomogenous { "id1": Key [keyDown "a", keyUp "a", pause] } LunaparkF.foo LunaparkF.performActions $ Object.fromHomogenous { "id1": Key [keyDown "a", keyUp "a", pause] }

spec https://github.com/jlipps/simple-wd-spec#perform-actions

but also supports deprecated json wire

wow this complicated

renaming is done in e60fd79

Yep, that's how it works. We had problems where ff driver and chromedriver were supporting only subsets of actual webdriver API.

src/Lunapark/Error.purs

src/Lunapark/Utils.purs

test/Main.purs

src/Lunapark/Error.purs

srghma · 2020-11-19T15:46:08Z

ok, I think that's all

cryogenian

I thing we agreed on all changes here 🎉

There is a couple of leftover comments and other tiny stuff.

cryogenian · 2020-11-19T15:50:43Z

packages.dhall

@@ -0,0 +1,111 @@
+{-


We don't need this comment I think

cryogenian · 2020-11-19T15:50:51Z

spago.dhall

@@ -0,0 +1,19 @@
+{-


cryogenian · 2020-11-19T15:51:26Z

src/Lunapark/Endpoint.purs

+post' ∷ String → Endpoint → Aff (Either LE.Error Json)
+post' uri ep = map handleAPIError $ N.post NR.json (uri <> print ep) Nothing
+
+-- | post_ ∷ String → Endpoint → Aff (Either LE.Error Unit)


Leftover comment

cryogenian · 2020-11-19T15:57:42Z

src/Lunapark/Error.purs

+  | CachingError CachingError
+
+printError :: Error -> String
+printError =


Please use unicode, → and ∷

The indetntation is kinda strange anyways Something like this should be aligned with other code

printError ∷ Error → String printError = case _ of JsonDecodeError jsonDecodeErrro → J.printJsonDecodeError jsonDecodeError WebDriverError webdriverError → "Response with error message:\n" <> " error type: " <> LWE.toStringCode webdriverError.error <> "\n" <> " message: " <> webdriverError.message <> "\n" <> " stacktrace: " <> webdriverError.stacktrace AffjaxError affjaxError → N.printError affjaxError CachingError cachingError → "Error during caching:\n " <> printCachingError cachingError where printCachingError = case _ of EmptyCases key → "There is no working implementation for " <> key <> " action." IncorrectCache key → "Trying another implementation for " <> key <> " action."

cryogenian · 2020-11-19T15:58:48Z

src/Lunapark/Types.purs

@@ -1,14 +1,14 @@
 -- | This module contains types for requests and response.
-- | Most of them are records with `α → Json` and `Json → Either String α` functions
+-- | Most of them are records with `α → J.Json` and `Json → Either J.JsonDecodeError α` functions


Auto replace? 🙂 The second Json should have J. or all of them shouldn't have J.

cryogenian · 2020-11-19T16:01:16Z

src/Lunapark/Error.purs

+import Lunapark.WebdriverError as LWE
+import Data.Argonaut.Decode (JsonDecodeError, printJsonDecodeError) as J
+
+data CachingError


cryogenian · 2020-11-19T16:03:18Z

bower.json

@@ -26,7 +26,7 @@
  ],
  "dependencies": {
    "purescript-affjax": "^9.0.0",


Not-updated dep

srghma · 2020-11-19T16:27:17Z

done

cryogenian · 2020-11-19T16:36:06Z

The build is failing

cryogenian · 2020-11-19T16:39:03Z

Thank you very much

cryogenian · 2020-11-19T16:41:18Z

Can you please squash commits to clean history up? I don't want to remove you from authors here

commit 3b7c111 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 18:36:44 2020 +0200 fix: ci commit 1831afa Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 18:15:29 2020 +0200 fix: review -> commments and style commit f94fc1a Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 17:41:22 2020 +0200 fix: review -> return type Lunapark commit 95ba053 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 17:36:30 2020 +0200 fix: review -> indentation commit e60fd79 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 17:34:32 2020 +0200 fix: review -> rename LunaparkActionsEffect to ActionsEffect commit b3abe29 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 17:11:39 2020 +0200 refactor: add comment commit e7428d9 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 16:38:53 2020 +0200 fix: review -> fine grain tryAndCache errors commit 89c37e4 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 12:12:16 2020 +0200 fix: review -> rename throwLeftJsonDecodeError to rethrowAsJsonDecodeError and remove throwLeftUserError commit 50f546e Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 12:06:00 2020 +0200 fix: review -> remove test commit 2c254f4 Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 12:05:31 2020 +0200 fix: review -> dont export handleAPIError commit 8ec2b5c Author: Serhii Khoma <srghma@gmail.com> Date: Thu Nov 19 11:47:25 2020 +0200 fix: review -> remove extra | commit 49e194e Author: Serhii Khoma <srghma@gmail.com> Date: Wed Nov 18 19:24:48 2020 +0200 feat: remove WithAction and WithLunapark, use rows commit 661bb41 Author: Serhii Khoma <srghma@gmail.com> Date: Wed Nov 18 19:04:42 2020 +0200 refactor: remove BaseRun and Lunapark, add LunaparkBaseEffects, LunaparkEffect, LunaparkActionsEffect commit c8dfbf1 Author: Serhii Khoma <srghma@gmail.com> Date: Wed Nov 18 13:27:30 2020 +0200 feat: rename post_ to post' to match post_ function from Affjax commit c29e049 Author: Serhii Khoma <srghma@gmail.com> Date: Tue Nov 17 15:01:20 2020 +0200 feat: update npm packages commit 69c22bb Author: Serhii Khoma <srghma@gmail.com> Date: Tue Nov 17 14:44:45 2020 +0200 feat: return bower commit 56eb0b4 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 20:36:00 2020 +0200 feat: printError commit a914049 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 20:34:41 2020 +0200 feat: printError commit 4a68045 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 19:14:14 2020 +0200 feat: travis -> use spago commit 8ef0769 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 19:09:15 2020 +0200 feat: remove bower commit 34d1fd8 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 19:09:07 2020 +0200 feat: add spago commit 42428e2 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 19:01:03 2020 +0200 feat: update bower commit 979439b Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 18:58:57 2020 +0200 feat: update to argonaut 7 and affjax 11 -> run ps-suggest -> fix prev run commit 5b58104 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 18:51:31 2020 +0200 feat: update to argonaut 7 and affjax 11 -> run ps-suggest commit 19df447 Author: Serhii Khoma <srghma@gmail.com> Date: Mon Nov 16 18:50:42 2020 +0200 feat: update to argonaut 7 and affjax 11

srghma · 2020-11-19T16:51:15Z

done

cryogenian reviewed Nov 18, 2020

View reviewed changes

cryogenian reviewed Nov 19, 2020

View reviewed changes

src/Lunapark/Error.purs Outdated Show resolved Hide resolved

cryogenian suggested changes Nov 19, 2020

View reviewed changes

cryogenian approved these changes Nov 19, 2020

View reviewed changes

srghma force-pushed the master branch from 3b7c111 to 8e3918f Compare November 19, 2020 16:50

cryogenian merged commit af9a883 into slamdata:master Nov 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update to argonaut 7 and affjax 11 #13

update to argonaut 7 and affjax 11 #13

srghma commented Nov 16, 2020

cryogenian commented Nov 16, 2020

srghma commented Nov 17, 2020

srghma commented Nov 18, 2020

cryogenian left a comment

cryogenian Nov 18, 2020

srghma Nov 19, 2020 •

edited

Loading

cryogenian Nov 19, 2020

srghma Nov 19, 2020

cryogenian Nov 18, 2020

srghma Nov 19, 2020

srghma Nov 19, 2020

cryogenian Nov 19, 2020 •

edited

Loading

srghma Nov 19, 2020

cryogenian Nov 19, 2020

srghma commented Nov 19, 2020

cryogenian left a comment

cryogenian Nov 19, 2020

cryogenian Nov 19, 2020

cryogenian Nov 19, 2020

cryogenian Nov 19, 2020

cryogenian Nov 19, 2020

cryogenian Nov 19, 2020

cryogenian Nov 19, 2020

srghma commented Nov 19, 2020

cryogenian commented Nov 19, 2020

cryogenian commented Nov 19, 2020

cryogenian commented Nov 19, 2020

srghma commented Nov 19, 2020

update to argonaut 7 and affjax 11 #13

update to argonaut 7 and affjax 11 #13

Conversation

srghma commented Nov 16, 2020

cryogenian commented Nov 16, 2020

srghma commented Nov 17, 2020

srghma commented Nov 18, 2020

cryogenian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srghma Nov 19, 2020 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryogenian Nov 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srghma commented Nov 19, 2020

cryogenian 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

srghma commented Nov 19, 2020

cryogenian commented Nov 19, 2020

cryogenian commented Nov 19, 2020

cryogenian commented Nov 19, 2020

srghma commented Nov 19, 2020

srghma Nov 19, 2020 •

edited

Loading

cryogenian Nov 19, 2020 •

edited

Loading