Provide updateActionModify API in AutoUpdate

[nanonaren]

Fixes #535

[snoyberg]

Travis builds failed due to (at least) missing imports.

[nanonaren]

How did that happen? Why would it compile for me?

[snoyberg]

You're probably using a newer version of GHC.

On Wed, May 4, 2016, 2:11 PM Naren Sundar notifications@github.com wrote:

How did that happen? Why would it compile for me?

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#547 (comment)

[nanonaren]

Right. I am using 7.10.3 and the Travis build doesn't show the missing import for <*> in that version but does in 7.8.4 and 7.6.3. How do I fix this?

[nanonaren]

I guess I'll just not use <*>. Probably the simplest thing to do.

[snoyberg]

By adding the missing import statement.

On Wed, May 4, 2016, 2:18 PM Naren Sundar notifications@github.com wrote:

Right. I am using 7.10.3 and the Travis build doesn't show the missing
import for <*> in that version but does in 7.8.4 and 7.6.3. How do I fix
this?

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#547 (comment)

[nanonaren]

Adding the import statement raises the complaint

/home/narens/Documents/wai/auto-update/Control/AutoUpdate.hs:42:1: Warning:
    The import of ‘Control.Applicative’ is redundant
      except perhaps to import instances from ‘Control.Applicative’
    To import instances alone, use: import Control.Applicative()

Is that okay?

[snoyberg]

It's OK. The usual solution is to use CPP to make the import conditional.

On Wed, May 4, 2016, 2:23 PM Naren Sundar notifications@github.com wrote:

Adding the import statement raises the complaint

/home/narens/Documents/wai/auto-update/Control/AutoUpdate.hs:42:1: Warning:
The import of ‘Control.Applicative’ is redundant
except perhaps to import instances from ‘Control.Applicative’
To import instances alone, use: import Control.Applicative()

Is that okay?

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#547 (comment)

[nanonaren]

Sorry, I could only find one example of #ifdef GHC_7_4 in the code. Would this be fine

#ifndef GHC_7_10
import           Control.Applicative     ((<*>))
#endif

[snoyberg]

I'm on my phone right now so I can't find an example myself, but there
should be lots of usages of MIN_VERSION scattered around

On Wed, May 4, 2016, 2:29 PM Naren Sundar notifications@github.com wrote:

Sorry, I could only find one example of #ifdef GHC_7_4 in the code. Would
this be fine

#ifndef GHC_7_10
import Control.Applicative ((<*>))
#endif

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#547 (comment)

[MaxGabriel]

This sentence switches tense—I think you want "to get the current value and update it if necessary."

[MaxGabriel]

Looks like there are some auto-update related compiler errors in the latest Travis results, btw: https://travis-ci.org/yesodweb/wai/jobs/127737868

[nanonaren]

It looks like this change cannot be made without breaking the API. So, this error

Network/Wai/Handler/Warp/Date.hs:31:27:

    Couldn't match type ‘()’ with ‘ByteString’

    Expected type: Control.AutoUpdate.UpdateSettings GMTDate

      Actual type: Control.AutoUpdate.UpdateSettings ()

    In the expression: defaultUpdateSettings

    In the first argument of ‘mkAutoUpdate’, namely

      ‘defaultUpdateSettings

         {updateAction = formatHTTPDate <$> getCurrentHTTPDate}’

happens because in the use above updateAction is updated to the type IO GMTDate but updateActionModify remains unchanged. It will have to be explicitly set.

[snoyberg]

That's unfortunate. I don't see a way of making this happen then.

[snoyberg]

There's another approach that could be taken I think: add a new function mkAutoUpdateWithModify that takes the modification value as a separate argument.

[MaxGabriel]

I think this is good to merge I think

[snoyberg]

Thanks!