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

Adds create-stack command #53

Merged
merged 1 commit into from
May 9, 2022
Merged

Adds create-stack command #53

merged 1 commit into from
May 9, 2022

Conversation

ryanmoran
Copy link
Member

Summary

Resolves #52

Also, moves top-level tests into integration directory.

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@ryanmoran ryanmoran requested a review from a team as a code owner April 29, 2022 18:12
@ryanmoran ryanmoran force-pushed the create-stack branch 2 times, most recently from d7fa313 to b7922f9 Compare April 29, 2022 18:24
@sophiewigmore
Copy link
Member

@ryanmoran I plan on giving this a full review tomorrow, just as an update!

@sophiewigmore
Copy link
Member

I have a small suggestion I want to hear your opinion on. I think that it might be nice to have a default vanilla stack descriptor file available with the command, or some kind of separate command that will give users a template. I understand that within the project we will largely be using this with our own stack descriptor files, but for the sake of ease of use for users that might want to leverage this tool themselves we should make it as easy as possible.

@sophiewigmore
Copy link
Member

Also, unrelated and probably not a big deal but I observed the packing process taking substantially longer now:

 ‣ time ./scripts/package.sh -v 1.2.3
real    0m40.948s
user    3m42.375s
sys     0m28.846s

This is versus the user 0m7.399s for the version on main. Just wanted to point this out and make sure this is expected.

@ryanmoran ryanmoran force-pushed the create-stack branch 2 times, most recently from 9c6dabd to dcefab4 Compare May 5, 2022 21:25
@ryanmoran
Copy link
Member Author

Also, unrelated and probably not a big deal but I observed the packing process taking substantially longer now:

 ‣ time ./scripts/package.sh -v 1.2.3
real    0m40.948s
user    3m42.375s
sys     0m28.846s

This is versus the user 0m7.399s for the version on main. Just wanted to point this out and make sure this is expected.

I'm not seeing this same slowdown. Can you confirm that this happens consistently with rebuilds? Is this from a fresh clone of the repo?

@ryanmoran
Copy link
Member Author

I have a small suggestion I want to hear your opinion on. I think that it might be nice to have a default vanilla stack descriptor file available with the command, or some kind of separate command that will give users a template. I understand that within the project we will largely be using this with our own stack descriptor files, but for the sake of ease of use for users that might want to leverage this tool themselves we should make it as easy as possible.

This sounds reasonable, but maybe requires a separate issue and PR?

@sophiewigmore
Copy link
Member

sophiewigmore commented May 9, 2022

I'm not seeing this same slowdown. Can you confirm that this happens consistently with rebuilds? Is this from a fresh clone of the repo?

It was a fresh clone of the repository, yes. But you're right subsequent builds are 11 seconds (when the build directory is removed before) which is much more reasonable!

@sophiewigmore
Copy link
Member

@ryanmoran issue filed for my suggestion: #56

@sophiewigmore
Copy link
Member

@ryanmoran Appreciate the documentation additions. Can you fix the merge conflicts? Then we can get this merged in!

- moves top-level tests into integration directory
@sophiewigmore sophiewigmore merged commit 459b587 into main May 9, 2022
@sophiewigmore sophiewigmore deleted the create-stack branch May 9, 2022 17:39
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.

Implement Stacks RFC0003: Stack Descriptor and Tooling
3 participants