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

446 project command #612

Merged
merged 88 commits into from
Nov 28, 2022
Merged

446 project command #612

merged 88 commits into from
Nov 28, 2022

Conversation

mtgoncalves1
Copy link
Contributor

No description provided.

@mtgoncalves1 mtgoncalves1 requested a review from mueschm July 7, 2022 16:32
@mtgoncalves1 mtgoncalves1 self-assigned this Jul 7, 2022
@mueschm
Copy link
Member

mueschm commented Jul 7, 2022

This is definitely on the right track but there are some changes we should make.

  1. Frontend, Backend, and Database are not the only types we allow. The point of these names is that if I ask for react for my project it should go to that template.json and get a list of elements to ask me for, that list could point to 10 other required service types all with unique names. This gives control over to the marketing and customer success team rather than having us update our code each time.
  2. The source of truth should not be in the project. We should just let users pick any project name and then look to see if that project exists on Github to kick off the flow.
  3. Looking at the way the databases work we may want to just allow custom architect.yml in the template.json file that just gets added directly into the generated architect.yml.

Copy link
Member

@mueschm mueschm left a comment

Choose a reason for hiding this comment

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

So there is some high level things we should fix first, once those are done I will do another deep dive pass since these require some big code changes.

src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/commands/project/create.ts Outdated Show resolved Hide resolved
Copy link
Member

@mueschm mueschm left a comment

Choose a reason for hiding this comment

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

This feels like we are getting very close!

src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
@zach-j-dev
Copy link
Contributor

@mueschm @tjhiggins Init command is ready to go and tested. As a quick note - put a PR for the config template , so the latest projects won't show in the selections until after it's merged (ASP, Ruby)

@zach-j-dev zach-j-dev requested review from TylerAldrich and removed request for mueschm and tjhiggins November 15, 2022 16:14
Copy link
Contributor

@TylerAldrich TylerAldrich left a comment

Choose a reason for hiding this comment

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

Couple small changes - also I noticed this has an empty creds.json committed. I think one of the tests always adds that empty file and we have it .gitignore'd in the tests/ directory, but it gets created wherever you run tests from... could you remove that file and add it to the .gitignore as well?

src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/architect/project/project.utils.ts Outdated Show resolved Hide resolved
src/commands/init.ts Outdated Show resolved Hide resolved
src/commands/init.ts Show resolved Hide resolved
@TylerAldrich
Copy link
Contributor

Code-wise everything looks good now, I just tried out the tool and noticed a couple things that are a bit misleading that we should reword:

? What should the name of the component be? test-comp

This doesn't seem to actually be the name of the component, but rather the folder that we create to clone the project into. I tested out cloning the Go project and I end up with a file structure like:

test-comp/
    go/
        architect.yml
        (other files...)

At the end, we then say:

To Deploy locally, run:
	$ architect dev test-comp/architect.yml

This isn't accurate because we create the test-comp folder, then clone the go project within it, so the real path would be architect dev test-comp/go/architect.yml (and would change for every project)

Perhaps what we should do to fix this is:

  1. Change "What should the name of the component be" to something that indicates it's just naming a folder (like "what's the name of your project") OR actually modify the cloned architect.yml with the name chosen (the former is easier of course, probably a good starting place because we don't need to be too fancy)
  2. We should run git clone <git repo url> . to clone into the folder we created so the file structure is:
test-comp/
    architect.yml
    (other files...)

With the updated clone command the final message won't be a lie and the dev command could be copy-pasta'd successfully

@TylerAldrich
Copy link
Contributor

Realized Zach was out today so I fixed the things I suggested above

@TylerAldrich TylerAldrich merged commit 45283a0 into rc Nov 28, 2022
@TylerAldrich TylerAldrich deleted the 446-project-command branch November 28, 2022 18:11
github-actions bot pushed a commit that referenced this pull request Nov 28, 2022
# [1.29.0-rc.5](v1.29.0-rc.4...v1.29.0-rc.5) (2022-11-28)

### Features

* **init:** Support creating projects from Architect templates ([#612](#612)) ([45283a0](45283a0)), closes [#695](#695)
@zach-j-dev
Copy link
Contributor

@TylerAldrich muchas gracias

github-actions bot pushed a commit that referenced this pull request Dec 1, 2022
# [1.29.0](v1.28.0...v1.29.0) (2022-12-01)

### Bug Fixes

* **cluster:** Updated examples to remove type flag. ([4e58979](4e58979))
* **dev:** Fix race condition inspecting containers that no longer exist, fixed bug with service_ref / full_service_name being incorrect, only log healthcheck once each time it happens ([#763](#763)) ([734356a](734356a))
* **register:** Default register path to ./architect.yml ([#765](#765)) ([01fa335](01fa335))
* **register:** Fix warning when dependencies are already registered ([#766](#766)) ([a593ea9](a593ea9))
* **validation:** validator for looser validation on account names ([#761](#761)) ([93d6859](93d6859))

### Features

* **dev:** loading a .env file for architect environment variables ([#753](#753)) ([dc0f0bc](dc0f0bc))
* **dev:** Log errors when liveness probe fails ([#759](#759)) ([0c65d19](0c65d19))
* **environment:create:** Warn when environment creation fails due to already existing environment ([#556](#556)) ([bb17f80](bb17f80))
* **init:** Support creating projects from Architect templates ([#612](#612)) ([45283a0](45283a0)), closes [#695](#695)
* **secrets:** Allow secrets from a remote env to be used in local development ([012e459](012e459))
@github-actions
Copy link

github-actions bot commented Dec 1, 2022

🎉 This PR is included in version 1.29.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants