Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4039 namespaced templates #4103

Merged
merged 17 commits into from Jun 24, 2018

Conversation

Projects
None yet
4 participants
@lylek
Copy link
Contributor

lylek commented Jun 21, 2018

  • stack new now allows template names of the form username/foo to download from a user other than commercialstack on Github, and can be prefixed with the servicegithub:, gitlab:, or bitbucket:.
  • Changelog.md has been updated
  • Help text for stack new has been updated
  • Tested this by temporarily adding a trace in the downloadTemplate function, to see the request it was using to download the template, and trying various new and old forms of the template name. Also created a local template and made sure that still took priority over a remote download.
  • I would have liked to add an integration test, but it's hard to test things that download from remote URLs. There isn't currently a way to stub that out.
  • Added documentation for the web site in the doc subdirectory. Unfortunately I'm not sure how to build that, to make sure it looks right.
@lylek

This comment has been minimized.

Copy link
Owner

lylek commented on src/Stack/Types/TemplateName.hs in 6bb37bb Jun 17, 2018

This line will break TEMPLATE_NAMEs in the form https://... because it will try to read them as repo requests, and, not matching "https", will default to Github. You should remove this line and move the definition of defaultRepoService back to New.hs. That's the place to do the defaulting.

johnmendonca and others added some commits Jun 18, 2018

@@ -1574,13 +1580,36 @@ Selected resolver: lts-3.2
Wrote project config to: /home/michael/my-yesod-project/stack.yaml
```

Alternatively you can use your own templates by specifying the path:
The default `stack-templates` repository is on [Github](https://github.com),

This comment has been minimized.

@snoyberg

snoyberg Jun 21, 2018

Contributor

How about adding a link directly to the repo?

This comment has been minimized.

@lylek

lylek Jun 22, 2018

Author Contributor

Sure, will do. There's also a direct link to the repo later in the section.

different Github user by prefixing the username and a slash:

```
stack new my-yesod-project yesodweb/yesod-simple

This comment has been minimized.

@snoyberg

snoyberg Jun 21, 2018

Contributor

I think it would be worth specifying exactly how this is resolved: user yesodweb, repo stack-templates, file yesod-simple.hsfiles.

This comment has been minimized.

@lylek

lylek Jun 22, 2018

Author Contributor

Yeah, that sounds good.

\ For example: foo or foo.hsfiles or ~/foo or\
\ https://example.com/foo.hsfiles")) <*>
help "Name of a template - can take the form\
\ [service:][username/]template with optional service name\

This comment has been minimized.

@snoyberg

snoyberg Jun 21, 2018

Contributor

Perhaps more accurate would be:

[[service:]username/]template

This comment has been minimized.

@lylek

lylek Jun 22, 2018

Author Contributor

No, actually you can write github:foo and it will pull the template foo from GitHub with the username commercialhaskell in the stack-templates repo. So either the service or the username can be dropped.

This comment has been minimized.

@snoyberg

snoyberg Jun 22, 2018

Contributor

Thanks for clarifying. In that case, I think it would be better to change the behavior, otherwise we'll be granting special status to the commercialhaskell account on other services when we don't necessarily control it.

This comment has been minimized.

@lylek

lylek Jun 22, 2018

Author Contributor

Yeah, I thought that might be odd too. I'll change the code.

parseRepoPathWithService service path =
case T.splitOn "/" path of
[user, name] -> Just $ RepoTemplatePath service user name
[name] -> Just $ RepoTemplatePath service defaultRepoUser name

This comment has been minimized.

@johnmendonca

johnmendonca Jun 23, 2018

Contributor

Deleting this line and defaultRepoUser should change the behavior as described above regarding commercialhaskell on other services.

This comment has been minimized.

@lylek

lylek Jun 23, 2018

Author Contributor

Not quite. That would prevent commercialhaskell being used as the default for other services. But it would also break the simple case where only a template name is supplied. Another function will have to be created to handle that case.

This comment has been minimized.

@johnmendonca

johnmendonca Jun 23, 2018

Contributor

Oh right, we end up here in the case where just a single name is given, thanks.

case sv of
Github -> [|RepoPath $ RepoTemplatePath Github u t|]
Gitlab -> [|RepoPath $ RepoTemplatePath Gitlab u t|]
Bitbucket -> [|RepoPath $ RepoTemplatePath Bitbucket u t|]

This comment has been minimized.

@johnmendonca

johnmendonca Jun 23, 2018

Contributor

I was wondering about this part, since I'm not clear about what it does exactly.

Could it do without the pattern match and just be:

RepoPath (RepoTemplatePath sv u t) -> [|RepoPath $ RepoTemplatePath sv u t|]

This comment has been minimized.

@lylek

lylek Jun 23, 2018

Author Contributor

No, that would cause TemplateHaskell to complain that there is no instance of something-or-other class. (Rep?) The alternative is to define an instance of that class, but Michael's recommendation was that it wasn't worth it.

lylek added some commits Jun 24, 2018

Changed default user assumption for stack new
Now no longer assumes the 'commercialhaskell' user for the location
of a new template unless the service is Github.
Updated stack new documentation
Clarifies how template names are resolved.
@lylek

This comment has been minimized.

Copy link
Contributor Author

lylek commented Jun 24, 2018

OK, I've adjusted the pull request as discussed. Well, one slight difference: it still accepts github:foo as an alternative to foo, defaulting to the commercialstack user, but will no longer accept gitlab:foo or bitbucket:foo with no username.

@snoyberg
Copy link
Contributor

snoyberg left a comment

LGTM! Thanks!

@snoyberg snoyberg merged commit facd588 into commercialhaskell:master Jun 24, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dbaynard dbaynard removed the needs changes label Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.