Skip to content

Fix #1276#1325

Merged
paf31 merged 2 commits intomasterfrom
1276
Aug 2, 2015
Merged

Fix #1276#1325
paf31 merged 2 commits intomasterfrom
1276

Conversation

@paf31
Copy link
Copy Markdown
Contributor

@paf31 paf31 commented Aug 2, 2015

@garyb Could you please review this? I also removed the used RebuildPolicy stuff from make.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Aug 2, 2015

This also simplifies the progress messages a little bit. The only ones which show up now are "Compiling X..." and "Writing Y" where Y is a .js file.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Aug 2, 2015

I'm not even sure the Writing... messages are needed.

@garyb
Copy link
Copy Markdown
Member

garyb commented Aug 2, 2015

👍

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Aug 2, 2015

@garyb Do you think the "Reading..." messages for externs are useful at all?

While we're at it, as a professional user, are there other progress messages you'd like to see?

@garyb
Copy link
Copy Markdown
Member

garyb commented Aug 2, 2015

I guess the "Reading" errors are useful for bugs in reading the externs files, but since we print filenames in the error messages anyway it's not important at all.

None of the status messages are that useful to me really when building projects, I only really pay attention to any error/warning messages we might throw out. The status messages are more of a debugging thing to me, when working on the language, but even at that I've not used them for a really long time.

@garyb
Copy link
Copy Markdown
Member

garyb commented Aug 2, 2015

Oh, did you see this too? purescript-contrib/pulp#52 (re: printing warnings to stderr)

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Aug 2, 2015

yeah, let's fix that before 0.7.2

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Aug 2, 2015

I've added it to this PR to just get it done.

@garyb
Copy link
Copy Markdown
Member

garyb commented Aug 2, 2015

👍

paf31 added a commit that referenced this pull request Aug 2, 2015
@paf31 paf31 merged commit fde48d7 into master Aug 2, 2015
@paf31 paf31 deleted the 1276 branch August 2, 2015 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants