search

unisonweb / base_v1

Unison base libraries, published using V1 codebase format
24 stars 14 forks source link

Adds List.intersperse #36

Closed dariooddenino closed 4 years ago

dariooddenino commented 4 years ago

This pull request adds List.intersperse. Hope everything's fine!

Code review

The changes summarized below are available for you to review, using the following command:

pull-request.load https://github.com/unisonweb/base:.trunk https://github.com/dariooddenino/unisoncode:.prs.base._intersperse

Updates:

 patch patch (added 1 updates)

Added definitions:

 List.intersperse                             : a -> [a] -> [a] (+1 metadata)
 List.intersperse.doc                         : Doc
 List.intersperse.tests.base                  : [Result] (+1 metadata)
 List.intersperse.tests.empty                 : [Result] (+1 metadata)
 List.metadata.authors.dariooddenino          : Author
 List.metadata.authors.dariooddenino.guid     : GUID
 List.metadata.copyrightHolders.dariooddenino : CopyrightHolder
 metadata.authors.dariooddenino               : Author
 metadata.authors.dariooddenino.guid          : GUID
 metadata.copyrightHolders.dariooddenino      : CopyrightHolder
 metadata.licenses.dariooddenino2020          : License (+1 metadata)
pchiusano commented 4 years ago

Cool! Can you make sure you’ve set your default author and license? The diff only has one metadata linked for the new definitions.

Instructions here: https://github.com/unisonweb/base/blob/master/CONTRIBUTING.md

dariooddenino commented 4 years ago

Umm I followed that guide and I was hoping everything was good :( I'll try again once I figure out how

dariooddenino commented 4 years ago

I have added DefaultMetadata = { prs.base = [".metadata.licenses.dariooddenino2020"] } to my .unisonCofig , but either it doesn't work retroactively, or I messed something up.

I then tried link .metadata.licenses.dariooddenino2020 .prs.base._intersperse.List.intersperse, which gave me what looked like a successful message, but when I check the diff with _base nothing changed from my pull request up here.

pchiusano commented 4 years ago

It’s not retroactive, though not sure why you’re not seeing any diff, unless it is already licensed.

I’ll take a look today. Could you just assign a license by commenting on this ticket. Just say “I hereby license the definitions listed in this PR using the MIT license” and then I can fix up during the merge. Thanks!

dariooddenino commented 4 years ago

I hereby license the definitions listed in this PR using the MIT license.

Sorry I wasn't able to fix it in the proper way :(

runarorama commented 4 years ago

Merged in base hash #si57j4rje9 🦄 🌈 🥇 . Thanks @dariooddenino !!