Skip to content

Remove psc-package, combine remaining executables#2632

Merged
paf31 merged 4 commits intomasterfrom
phil/2604
Feb 8, 2017
Merged

Remove psc-package, combine remaining executables#2632
paf31 merged 4 commits intomasterfrom
phil/2604

Conversation

@paf31
Copy link
Copy Markdown
Contributor

@paf31 paf31 commented Feb 7, 2017 •
edited
Loading

Fixes #2604, #2618 and #2336.

I created a separate repo for psc-package at https://github.com/purescript/psc-package/.

@paf31 paf31 requested review from garyb and hdgarrood February 7, 2017 04:11
@hdgarrood
Copy link
Copy Markdown
Contributor

hdgarrood commented Feb 7, 2017

I think this only partially addresses #2604 - we also need to remove hardcoded bower paths from psc-publish and maybe other compiler tools. But perhaps it would be better to make a separate issue for that.

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Feb 7, 2017

@hdgarrood There is #2517 to track the work to remove Bower. I'll merge all of these together now. Thanks.

@paf31
Single consolidated executable (#2633)
196d86f 
* Single consolidated executable #2336

* PSCi

* bundle

* docs

* hierarchy

* publish

* ide

* Remove some old options, fix #2618

* Fix warning

* fix test failure and cabal warning (#2634)

* fix test failure and cabal warning

* add missing files to cabal file
@paf31 paf31 changed the title Remove psc-package Remove psc-package, combine remaining executables Feb 7, 2017
@paf31 paf31 requested a review from kritzcreek February 7, 2017 17:32
@hdgarrood
Copy link
Copy Markdown
Contributor

hdgarrood commented Feb 7, 2017

purs sounds good to me, especially since a lot of what it does isn't really compiling. Also just to check, are we intending to remove these wrapper scripts at the next major release?

kritzcreek
kritzcreek approved these changes Feb 7, 2017
View reviewed changes
app/Command/IDE.hs Outdated
{-# LANGUAGE NoImplicitPrelude #-}

module Main where
module Command.IDE (command) where
Copy link
Copy Markdown
Member

@kritzcreek kritzcreek Feb 7, 2017

Choose a reason for hiding this comment

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

Would prefer Ide like in the rest of the codebase, just for consistencies sake.

Copy link
Copy Markdown
Contributor Author

@paf31 paf31 Feb 7, 2017

Choose a reason for hiding this comment

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

Sounds good

@hdgarrood
Copy link
Copy Markdown
Contributor

hdgarrood commented Feb 7, 2017

You've checked the wrapper scripts item in your checklist but I can't find any wrapper scripts in this PR's diff, am I missing something?

@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Feb 7, 2017

Sorry, forgot to push my changes.

@paf31 paf31 closed this Feb 7, 2017
@paf31 paf31 reopened this Feb 7, 2017
@hdgarrood
Copy link
Copy Markdown
Contributor

hdgarrood commented Feb 7, 2017

Looks great :) perhaps we should consider wrapper scripts for Windows too? I think we want e.g. psc.cmd with just a single line, purs compile %* — apparently the Windows analogue for $@ is %*.

@hdgarrood
Copy link
Copy Markdown
Contributor

hdgarrood commented Feb 7, 2017

If anyone has a Windows environment around to check that that works, that would be handy.

@kritzcreek kritzcreek mentioned this pull request Feb 8, 2017
Merged
@hatashiro hatashiro mentioned this pull request Feb 8, 2017
Closed
@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Feb 8, 2017

Shall I merge this then? I guess if we want to make a 0.10.7 then we can branch from the tag, yes?

@hdgarrood
Copy link
Copy Markdown
Contributor

hdgarrood commented Feb 8, 2017

Yep, sounds good to me.

@paf31 paf31 merged commit 74ca00d into master Feb 8, 2017
@paf31 paf31 deleted the phil/2604 branch February 8, 2017 17:49
@paf31
Copy link
Copy Markdown
Contributor Author

paf31 commented Feb 8, 2017

Thanks everyone.

@hatashiro hatashiro mentioned this pull request Feb 10, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garyb garyb garyb approved these changes

@kritzcreek kritzcreek kritzcreek approved these changes

+1 more reviewer

@hdgarrood hdgarrood hdgarrood approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status: ready for review type: breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@paf31 @hdgarrood @garyb @kritzcreek