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

Spm #30

Merged
merged 27 commits into from
Jun 21, 2016
Merged

Spm #30

merged 27 commits into from
Jun 21, 2016

Conversation

finestructure
Copy link
Member

Fixes #24

I just realised that I hadn't opened a PR for this yet :/

I've merged in latest changes from master just now. This will continue to be tricky and generate merge conflicts every time due to the way vapor.swift is no more in this branch.

It would be great if we could merge this next so we can avoid the tricky merges from master. The only thing that still needs changing is reviewing the install/update process in the spm context, i.e. by downloading the bootstrap script and running it instead of downloading the vapor script and compiling it. This should be simple and I'll look into that once soon as we're sure we're going down this path.

See #24 for more details.

@tanner0101
Copy link
Member

tanner0101 commented Jun 13, 2016

Had to push out the last update, but I won't push anymore to master. Let's get this straightened out this week. :)

There's also some good console code added to Vapor that might be worth putting in a separate package so we can use it in both places.

@finestructure
Copy link
Member Author

Haha, in WWDC week? Let's see how that goes ;) I'll try and fix up the remaining bits tomorrow.

@finestructure
Copy link
Member Author

@tannernelson just looking into merging 0.5.3 into this branch I noticed that there seems to have been an issue with installing that made you bring the install command back: 8f47853. I’m going to drop this as part of the merge because this bit doesn’t really make sense with the bootstrap.swift script but it would be good to understand what the problem was so I don’t re-introduce it.

Have you got a problem description that I could test against? Thanks!

Sven A. Schmidt and others added 6 commits June 14, 2016 16:13
# Conflicts:
#	vapor.swift
…ptionally) install to a location (i.e. -> `/usr/local/bin/vapor`, which is the default) instead of a directory. This way we can run it as per installation instructions.

Added FIXME warning to change the branch to `master` before merge.
…tead of `vapor self update`

( since there's now only one `self` command left)
- Updated `.travis.yml` to reflect installation instructions and changes to `update` command
This can only work after cli.qutheory.io starts serving bootstrap.swift
@finestructure
Copy link
Member Author

finestructure commented Jun 15, 2016

@tannernelson I’m done with this but there are two things to do after merging.

  1. The branch where bootstrap.swift pulls from is set to this branch, spm, right now, in order to make the tests pass. We’ll either need to change that or make it a parameter, defaulting to master. Just keeping the branch name in there is easier for now, otherwise we’d need to do more parameter parsing.
  2. I’ve had to disable the vapor update test, because it cannot work until we’re serving bootstrap.swift from cli.qutheory.io. Actually, it may be better to change to a new URL, for example vapor-cli.qutheory.io, and switch over before merging. That way old clients won’t fall over.

@finestructure
Copy link
Member Author

PS: Changes to be made are FIXME labeled

@tanner0101
Copy link
Member

I'm going to merge this for now so we don't make any more accidental changes.

But I have a few things I want to add before we tag. Like support for Vapor's commands.

Nice work on this @feinstruktur!

@tanner0101 tanner0101 merged commit c95d1bd into master Jun 21, 2016
@tanner0101 tanner0101 deleted the spm branch June 21, 2016 00:31
@finestructure
Copy link
Member Author

Thanks @tannernelson :) Not sure what exactly you mean by "Vapor's commands" but if you've got some specifics I can find some time this week to look into it. Now that this is in, I was also going to look into adding some unit tests via swift test.

@tanner0101
Copy link
Member

Unit tests would be awesome. The Vapor commands can be read about in the docs here: https://vapor.readme.io/v0.12/docs/command

Basically Vapor has a little bit of CLI functionality itself now for serving and running preparations (migrations).

You run this by doing .build/debug/App serve .build/debug/App prepare --revert etc.

I want to make the CLI reduce the typing here by doing vapor run serve

To accomplish this we would need to parse the Package.swift to figure out the name of the package, then run the executable from either the debug or release folders.

so something like vapor run serve --release would translate to .build/release/MyAppName serve

@tanner0101
Copy link
Member

Also if you look at the console stuff powering that, you can see there's some cool code here for doing colors, etc.

I think it could be worth breaking this out into it's own package so both Vapor and the CLI can use it: https://github.com/qutheory/vapor/tree/master/Sources/Vapor/Console

@finestructure
Copy link
Member Author

Cheers, I've created tickets #32, #33, and #34 for those

tanner0101 pushed a commit that referenced this pull request Jul 17, 2017
…ver_kind

Removing database server kind description
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