[#227] Create `Relude.Extra` module by mstruebing · Pull Request #236 · kowainik/relude

mstruebing · 2019-12-16T14:18:55Z

This is currently WIP as there are still errors while building.
Maybe someone can help me with that? I don't know how to fix these:

The class method signature for ‘...’ lacks an accompanying binding

or

‘...’ is not a (visible) method of class ‘...’

Resolves #227

Checklist:

HLint

I've changed the exposed interface (add new reexports, remove reexports, rename reexported things, etc.).
- I've updated hlint.dhall accordingly to my changes (add new rules for the new imports, remove old ones, when they are outdated, etc.).
- I've generated the new .hlint.yaml file (see this instructions).

General

I've updated the CHANGELOG with the short description of my latest changes.
All new and existing tests pass.
I keep the code style used in the files I've changed (see style-guide for more details).
I've used the stylish-haskell file.
My change requires the documentation updates.
- I've updated the documentation accordingly.
I've added the [ci skip] text to the docs-only related commit's name.

This is currently WIP as there are still errors while building. Maybe someone can help me with that? I don't know how to fix these: ``` The class method signature for ‘...’ lacks an accompanying binding ``` or ``` ‘...’ is not a (visible) method of class ‘...’ ``` closes kowainik#227

hint-man

Do you know why your PR is still not approved? Because I chose not to approve it. But they will.

src/Relude/Extra.hs

Co-Authored-By: hint-man[bot] <44720633+hint-man[bot]@users.noreply.github.com>

chshersh

@mstruebing Thanks for this refactoring! I believe we see compilation error because you wrote imports explicitly like this:

import Relude.Monoid (Monoid, Semigroup)

I guess the error should go if you write it like this (by exporting not only typeclasses but also their methods):

import Relude.Monoid (Monoid (..), Semigroup (..))

But I don't think that any of these should be neccessary. Implicit import Relude should work. You only need to add the Relude.Extra module to the .cabal file.

src/Relude/Extra.hs

chshersh · 2019-12-18T10:30:21Z

src/Relude/Extra/Bifunctor.hs

+import Relude.Functor (Bifunctor, Functor, second, fmap, first)
+import Relude.Function ((.))
+import Relude.Extra.Validation (bimap)


We've been using implicit import Relude in extra modules because Relude doesn't reexport extra modules. So it's fine to save some time for us here and don't specify each module explicitly.

It's not as easy possible as there are many cycles - Relude imports Relude.Extra which imports Relude.Extra.X which imports Relude. I'll try :)

But Relude mustn't export Relude.Extra. And it currently doesn't do this:

https://github.com/kowainik/relude/blob/master/src/Relude.hs

The main motivation behind having Relude.Extra modules is that they are not exported by default from Relude.

Thank you, now it is building and tests are running through.
I've needed to remove {-# LANGUAGE Safe #-} from Extra.hs otherwise I got errors:

src/Relude/Extra.hs:31:1: error: Relude.Extra.Group: Can't be safely imported! The module itself isn't safe. | 31 | import Relude.Extra.Group | ^^^^^^^^^^^^^^^^^^^^^^^^^ src/Relude/Extra.hs:33:1: error: Relude.Extra.Map: Can't be safely imported! The module itself isn't safe. | 33 | import Relude.Extra.Map

mstruebing · 2019-12-18T14:50:00Z

If anything is alright, could someone help me with the open checkboxes of the PR template, do I need to do anything specific? :)

vrom911

Thanks for your work!

Regarding the leftover check-boxes: You don't need to do anything with HLint as you didn't added any main modules' functions. All other boxes are done 👍

vrom911 · 2019-12-18T14:53:43Z

src/Relude.hs

+* __"Relude.Extra.Newtype"__: generic functions that automatically work for any	
+  @newtype@.	
+* __"Relude.Extra.Tuple"__: functions for working with tuples.	
+* __"Relude.Extra.Type"__: functions for inspecting and working with types.	


Some extra white spaces are added accidentally, I guess.

vrom911 · 2019-12-18T14:55:52Z

src/Relude.hs


+{- $extra
+__"Relude.Extra"__ contains reexports from "Relude.Extra".
+-}


This section is not used anywhere in Haddocks, so it won't be visible. Instead, you can add the link to Relude.Extra module with your short description right above the whole list of the extra modules ⬆️

vrom911 · 2019-12-18T14:56:40Z

src/Relude/Extra/Enum.hs


 import Relude
-import Relude.Extra.Tuple (mapToFst)
+import Relude.Extra.Tuple


It's okau to leave explicit list of imports for extra modules 🙂

vrom911 · 2019-12-18T14:57:13Z

src/Relude/Extra/Foldable.hs


 import Relude

-


We actually use 2 lines break after the imports section in our style guide

vrom911 · 2019-12-18T14:59:26Z

CHANGELOG.md

+
 `relude` uses [PVP Versioning][1].
 The changelog is available [on GitHub][2].



Thanks for updating CHANGELOG!

We usually create the new section right above the latest release (if there is no any).

So you can add it like this on this line:

## Unreleased * [#227](https://github.com/kowainik/relude/pull/236): Create `Relude.Extra` module (by [@mxstrbng](https://github.com/mstruebing))

mstruebing · 2019-12-18T15:20:37Z

Thanks @vrom911 I've adjusted the code to your mentioned changes and hope everything is fine now. If not, please let me know.

chshersh

Thanks @mstruebing, looks nice!

vrom911

Yes, looks nice! Thank you again, @mstruebing 👍

mstruebing requested review from chshersh and vrom911 as code owners December 16, 2019 14:18

hint-man bot reviewed Dec 16, 2019

View reviewed changes

src/Relude/Extra.hs Outdated Show resolved Hide resolved

Update src/Relude/Extra.hs

74d8de9

Co-Authored-By: hint-man[bot] <44720633+hint-man[bot]@users.noreply.github.com>

chshersh reviewed Dec 18, 2019

View reviewed changes

mstruebing changed the title ~~WIP [#227] Create Relude.Extra module~~ [#227] Create Relude.Extra module Dec 18, 2019

mstruebing added 2 commits December 18, 2019 15:49

Add changelog

42589eb

Dont export Relude.Extra

bdf8e7d

mstruebing force-pushed the 227/reludeExtra branch from e036ac6 to bdf8e7d Compare December 18, 2019 14:49

vrom911 reviewed Dec 18, 2019

View reviewed changes

Adjust to review comments

b9a31ae

chshersh approved these changes Dec 18, 2019

View reviewed changes

vrom911 approved these changes Dec 19, 2019

View reviewed changes

vrom911 merged commit 02f95b3 into kowainik:master Dec 19, 2019

chshersh added this to the v0.7.0.0: Refiner milestone Feb 25, 2020


		`relude` uses [PVP Versioning][1].
		The changelog is available [on GitHub][2].

Uh oh!

Conversation

mstruebing commented Dec 16, 2019 • edited by vrom911 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist:

HLint

General

Uh oh!

hint-man bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chshersh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstruebing commented Dec 18, 2019

Uh oh!

vrom911 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstruebing commented Dec 18, 2019

Uh oh!

chshersh left a comment

Choose a reason for hiding this comment

Uh oh!

vrom911 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mstruebing commented Dec 16, 2019 •

edited by vrom911

Loading