Skip to content

Implement safe at (!!) function#204

Merged
vrom911 merged 8 commits intokowainik:masterfrom
kutyel:safe-at
Oct 7, 2019
Merged

Implement safe at (!!) function#204
vrom911 merged 8 commits intokowainik:masterfrom
kutyel:safe-at

Conversation

@kutyel
Copy link
Copy Markdown
Contributor

@kutyel kutyel commented Oct 7, 2019 •
edited
Loading

Resolves #201

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.

@kutyel
Copy link
Copy Markdown
Contributor Author

kutyel commented Oct 7, 2019

Sorry about the failing compilation 🤦‍♂ I'm fairly new to Nix and have zero experience with Dhall, so I didn't know how to locally compile and run the tests of the project 😅 Hope you can give me some hints! Thanks in advance for any feedback on this 😉

vrom911
vrom911 reviewed Oct 7, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kutyel no worries!

Don't worry about Nix part, just use familiar way to build and test the project.

Regarding Dhall, we are only using it for HLint rules generation, and as long as you don't add new rules in there, you don't have to install that. We have the HLint check on CI, so, in case of anything, it will be caught there.

chshersh
chshersh reviewed Oct 7, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kutyel Good job! I left some comments and suggestions, but the start is good 👍

kutyel
kutyel commented Oct 7, 2019
View reviewed changes
@kutyel kutyel requested review from chshersh and vrom911 October 7, 2019 11:41
@chshersh chshersh added Hacktoberfest https://hacktoberfest.digitalocean.com/ new Bring something new into library (add function or type or interface) labels Oct 7, 2019
chshersh
chshersh reviewed Oct 7, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work so war! There're some more improvements but they are mostly stylistic 🙂

@kutyel kutyel requested a review from chshersh October 7, 2019 15:41
chshersh
chshersh approved these changes Oct 7, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kutyel Thanks for your contribution! ❤️ Looks great 👍

@kutyel
Copy link
Copy Markdown
Contributor Author

kutyel commented Oct 7, 2019

Thank you both for letting me contribute and for your great feedback! 🙌

vrom911
vrom911 approved these changes Oct 7, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Thanks a lot for your time and contribution 🥇

@vrom911 vrom911 merged commit 9b7b273 into kowainik:master Oct 7, 2019
@kutyel kutyel deleted the safe-at branch October 7, 2019 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vrom911 vrom911 vrom911 approved these changes

+1 more reviewer

@chshersh chshersh chshersh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kutyel kutyel

Labels

Hacktoberfest https://hacktoberfest.digitalocean.com/ new Bring something new into library (add function or type or interface)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add safe at (!!) function

3 participants

@kutyel @chshersh @vrom911