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

Seperate server functionality into server feature #130

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

karthik2804
Copy link
Contributor

@karthik2804 karthik2804 commented Sep 14, 2022

Split functionality into server feature as per #129. The actual issue arose due to handlebars-sprig now having a dependency on spin-sdk to enable the tweet embeds functionality. This has now been fixed.

Tested compiling on both windows 10 VM as well as cross-compiling using the x86_64-pc-windows-gnu and x86_64-pc-windows-msvc target.

Signed-off-by: Karthik Ganeshram karthik.ganeshram@fermyon.com

Signed-off-by: Karthik Ganeshram <karthik.ganeshram@fermyon.com>
@itowlson
Copy link
Contributor

@karthik2804 Thanks for figuring out the real cause and for fixing it! It looks good to me, though probably needs someone more knowledgeable than me to provide proper feedback.

@tpmccallum Could you give this a go please? I'm thinking exercise the Windows client but also re-exercise the Linux or Mac client, and the server, to make sure this hasn't affected them. Thanks!

@tpmccallum
Copy link
Contributor

Thanks @itowlson, thanks, I will test Linux, macOS and Windows today and respond in here.
Also, thanks @karthik2804 great work finding and updating this for us, much appreciated.

@tpmccallum
Copy link
Contributor

Testing building Bartholomew Server and Bartholomew CLI using the new code in this PR.

Housekeeping

``bash
gh pr checkout 130


a.k.a.

```bash
git pull origin pull/130/head

Bartholomew Server

macOS

make build

// ... 

Finished release [optimized] target(s) in 1m 00s

Windows

make build

// ... 

Finished release [optimized] target(s) in 1m 13s

Linux

make build

// ... 

Finished release [optimized] target(s) in 52.92s

Bartholomew CLI

macOS

make bart 
cargo build --release --manifest-path=bart/Cargo.toml

// ... 

 Finished release [optimized] target(s) in 40.75s

Windows

make bart
cargo build --release --manifest-path=bart/Cargo.toml
Finished release [optimized] target(s) in 1m 01s

Linux

make bart
cargo build --release --manifest-path=bart/Cargo.toml

// ... 
    
Finished release [optimized] target(s) in 1m 00s

LGTM

@tpmccallum
Copy link
Contributor

I also used bart check to confirm it works as intended (i.e. check the contents of blog pages).
Then also in addition placed the new bartholomew.wasm file in the bartholomew-site-template's module directory and started the CMS using spin up --follow-all.
This all worked perfectly.
We are good to go, I believe.

@itowlson
Copy link
Contributor

@tpmccallum Perfect, thanks!

Copy link
Contributor

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

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.

3 participants