-
Notifications
You must be signed in to change notification settings - Fork 1
feat: TT-509 Use terraform to provision ui infraestructure #819
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good. Please add the readme.md file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. keep in mind that you should also request a review from Fausto.
* feat: TT-580 Missing placeholders * feat: TT-580 add placeholders in ticket uri and description text inputs on the time clock page Co-authored-by: Luis Moyon <[email protected]>
* feat: TT-578 improve the style of the form element
…ck out button (#824) * style: TT-582 improve the summary section with a new row with information of actual clock in register and clock out button * fix: TT-582 add missing unit test to confirm that clockout function in TimeEntriesSummaryComponent emits an event * style: TT-582 add a border radius to current elapsed time box Co-authored-by: David Cadena <[email protected]>
* feat: TT-576 Improve grids visual presentation and function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job my friend!
Dockerfile
Outdated
| RUN touch /var/run/nginx.pid && chown -R ${USERNAME}:${USERNAME} /var/run/nginx.pid | ||
|
|
||
| USER ${USERNAME} | ||
| #USER ${USERNAME} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is not needed anymore you can delete it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Could your add a FIXME statement explaining why this has been commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed with few comments
* feat: TT-577 get profile pic from Google account * feat: TT-577 get profile pic from Google account * feat: TT-577 get profile pic from Google account Co-authored-by: alejandra-ponce <[email protected]>
This reverts commit 3bee7e3. Co-authored-by: Carlos Carvajal <[email protected]>
Dockerfile
Outdated
| RUN touch /var/run/nginx.pid && chown -R ${USERNAME}:${USERNAME} /var/run/nginx.pid | ||
|
|
||
| USER ${USERNAME} | ||
| #USER ${USERNAME} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Could your add a FIXME statement explaining why this has been commented out?
infrastructure/main.tf
Outdated
| } | ||
| } | ||
|
|
||
| data "azurerm_container_registry" "registry" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: I think this will make another unnecessary call to the azure API. The data.terraform_remote_state.service.outputs.container_registry_name already provides a valid reference to the container registry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really? i change that
infrastructure/main.tf
Outdated
| } | ||
|
|
||
| data "azurerm_resource_group" "root" { | ||
| name = data.terraform_remote_state.service.outputs.resource_group_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: Same issue as the one explained above
infrastructure/variables.tf
Outdated
| @@ -0,0 +1,17 @@ | |||
| variable "docker_image_name" { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: replace this with image_tag, something like https://github.com/ioet/time-tracker-v2/blob/255112b0f270ae17de84a20cd7791705dc573f8e/infrastructure/variables.tf#L7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the docker_image_name i pass the name of the image and the tag this is because the module is configure in that way if i only put tag. I think can we drop in a confusion because tag is (:2.0.1) and docker_image for me its (timetracker:2.0.1)
* style: TT-582 improve the summary section with a new row with information of actual clock in register and clock out button * fix: TT-582 add missing unit test to confirm that clockout function in TimeEntriesSummaryComponent emits an event * style: TT-582 add a border radius to current elapsed time box * style: TT-582 improved the clocked in hour text and current time box * style: TT-582 improved the clocked in hour text and current time box * style: TT-582 removed whitespace Co-authored-by: Luis Moyón <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now
|
Kudos, SonarCloud Quality Gate passed!
|
Codecov Report
@@ Coverage Diff @@
## master #819 +/- ##
==========================================
+ Coverage 95.64% 97.48% +1.84%
==========================================
Files 107 107
Lines 2273 2430 +157
Branches 175 203 +28
==========================================
+ Hits 2174 2369 +195
+ Misses 54 23 -31
+ Partials 45 38 -7
Continue to review full report at Codecov.
|








Create the infraestructure folder with the 4 main files
(main.tf, prod.tfvars, stage.tfvars, variables.tf) and use
the organization private modules to deploy all the resources to
the ui need to deploy in the two enviroments (stage and prod)
Resolves: https://ioetec.atlassian.net/browse/TT-509