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

Update vue scaffolding #137

Merged
merged 2 commits into from
Nov 21, 2023
Merged

Conversation

emhoracek
Copy link
Contributor

@emhoracek emhoracek commented Oct 25, 2023

I ran into some trouble trying out the tutorial forum example using the Vue scaffolding instead of svelte, so I tried to fix it. I couldn't figure out a way to easily try out my fixes without making an option to not make a nix environment, which is why I added the setup_nix option, but I can revert that if it's not useful. I think I got all the places that using error.data.data, but there are other places that have the extra Hash (making postHashHash instead of postHash).

@pdaoust
Copy link
Collaborator

pdaoust commented Oct 26, 2023

Thanks for submitting this @emhoracek ! I discovered the same thing when I was doing a QA sweep of the scaffold templates (using the same technique you did -- following the tutorial with another front-end framework). The fact that you discovered and fixed the same issues suggests that my PR didn't get merged... hm

Looking at your PR, it also looks like you discovered some errors in error display cases (ironic) that I never hit. So I'd like to merge your work here...

Almost all of it, that is! looks like you've also added the option to scaffold holonix env for the example hApps, which is valuable on its own. Could you split that into a separate PR?

@emhoracek
Copy link
Contributor Author

Almost all of it, that is! looks like you've also added the option to scaffold holonix env for the example hApps, which is valuable on its own. Could you split that into a separate PR?

Sure, just made #138

I'm going to go again and fix the other places that have that HashHash bug as well.

@ThetaSinner
Copy link
Member

Thank you for this change @emhoracek, I'm a Vue user myself and appreciate bug fixes going into it because Svelte seems to get a lot of the love :) I will give this a run early next week to check it and come back with a review

@ThetaSinner ThetaSinner added ShouldBackport/0.1 This change should be backported to develop-0.1 ShouldBackport/0.2 This change should be backported to develop-0.2 labels Nov 3, 2023
@ThetaSinner
Copy link
Member

It took me a bit of time to figure out what I thought was a bug with scaffolding while testing this. Turns out it was user error on my part :) Giving this a look now

@ThetaSinner
Copy link
Member

Looks great, thank you!

@ThetaSinner ThetaSinner merged commit 6e67556 into holochain:develop Nov 21, 2023
1 check passed
ThetaSinner added a commit that referenced this pull request Nov 28, 2023
ThetaSinner added a commit that referenced this pull request Nov 28, 2023
@ThetaSinner ThetaSinner removed ShouldBackport/0.2 This change should be backported to develop-0.2 ShouldBackport/0.1 This change should be backported to develop-0.1 labels Nov 28, 2023
c12i pushed a commit to c12i/scaffolding that referenced this pull request Jan 11, 2024
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