Skip to content

Conversation

@earlbread
Copy link
Contributor

This patch adds the description field to package metadata. (#100)

@kanghyojun kanghyojun added the typ:enhance Type: Enhancement/new feature label Aug 21, 2017
Copy link
Member

@dahlia dahlia left a comment

Choose a reason for hiding this comment

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

@earlbread Everything looks good to me except for some trivial style things. 👍🏼

import Nirum.Package (Package (metadata, modules), resolveBoundModule)
import Nirum.Package.Metadata ( Author (Author, email, name, uri)
, Metadata (Metadata, authors, target, version)
, Metadata (Metadata
Copy link
Member

Choose a reason for hiding this comment

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

A space should be inserted after the opening parenthesis (so that parentheses are vertically aligned with commas) e.g.:

, Metadata ( Metadata
           , authors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll update it. Thank you!

, types
)
import Nirum.Package.Metadata ( Metadata (Metadata, authors, target, version)
import Nirum.Package.Metadata ( Metadata (Metadata
Copy link
Member

Choose a reason for hiding this comment

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

A space should be inserted after the opening parenthesis (so that parentheses are vertically aligned with commas) e.g.:

, Metadata ( Metadata
           , authors

pVersion = SV.toText $ version metadata'
pDescription :: Code
pDescription = case description metadata' of
Just value -> T.intercalate "" ["'", value, "'"]
Copy link
Member

Choose a reason for hiding this comment

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

It can be broken if value contains any apostrophe characters (i.e. single quotes).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! How about this change?

pDescription = case description metadata' of
                  Just value -> stringLiteral value

Copy link
Member

Choose a reason for hiding this comment

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

It's why stringLiteral function exists. Seems correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I'll fix like above. Thank you!

)
import Nirum.Package.Metadata ( Author (Author, name, email)
, Metadata (Metadata, authors, target, version)
, Metadata (Metadata
Copy link
Member

Choose a reason for hiding this comment

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

A space should be inserted after the opening parenthesis (so that parentheses are vertically aligned with commas) e.g.:

, Metadata ( Metadata
           , authors

StandaloneDeriving, TypeFamilies #-}
module Nirum.Package.Metadata ( Author (Author, email, name, uri)
, Metadata (Metadata, authors, target, version)
, Metadata (Metadata
Copy link
Member

Choose a reason for hiding this comment

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

A space should be inserted after the opening parenthesis (so that parentheses are vertically aligned with commas) e.g.:

, Metadata ( Metadata
           , authors

@earlbread
Copy link
Contributor Author

earlbread commented Aug 22, 2017

@dahlia I fixed things you mentioned. Thank you!

Copy link
Member

@dahlia dahlia left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dahlia dahlia merged commit 33c5129 into nirum-lang:master Aug 22, 2017
This was referenced Aug 22, 2017
@dahlia dahlia added cat:packaging Category: Nirum schema packaging (not compiler packaging) cmp:frontend Component: Compiler frontend (e.g., CLI, parser, AST) labels Aug 26, 2017
@earlbread earlbread deleted the add_description_metadata branch August 29, 2017 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:packaging Category: Nirum schema packaging (not compiler packaging) cmp:frontend Component: Compiler frontend (e.g., CLI, parser, AST) typ:enhance Type: Enhancement/new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants