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
feat: sonarqube tasks added to pipeline #8
Conversation
The pipeline has been updated to include the sonarqube tasks Dev dependency nyc has been included Script test:report has been added Dockerignore and gitignore updated Renaming of the docker build task
@@ -2,6 +2,6 @@ version: '3' | |||
|
|||
services: | |||
service: | |||
image: ilwebshops.azurecr.io/infinitaslearning/<%= name %>:latest | |||
image: ilwebshops.azurecr.io/infinitaslearning/<%= name %>:latest # Change to the proper 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.
maybe should the registry being an env var or a new var requested when you launch yeoman?
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.
You're right it should be. However, I think it is not part of the scope of this PR
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.
I left the comment there because it was hardcoded and not clear to see that it must be changed. We could do a new var request while making the template on a new PR, sounds good to me
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.
I'm ok with the comment.
displayName: 'sonarqube: publish analysis' | ||
inputs: | ||
pollingTimeoutSec: '300' | ||
|
||
- task: Docker@2 | ||
displayName: push |
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.
docker: push image
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.
@w3dani @dustytrinkets I think we also have to validate if it is in master or not as we will get errors in the PR.
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.
lets include it then
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.
Just add pr: none
to azure-pipeline.yaml, that will prevent to run build on PR merge requests. No need to filter by branch.
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
@@ -2,6 +2,6 @@ version: '3' | |||
|
|||
services: | |||
service: | |||
image: ilwebshops.azurecr.io/infinitaslearning/<%= name %>:latest | |||
image: ilwebshops.azurecr.io/infinitaslearning/<%= name %>:latest # Change to the proper 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.
I'm ok with the comment.
Type:
What's the focus of this PR:
The pipeline has been updated to include the sonarqube tasks
Dev dependency nyc has been included
Script test:report has been added
Dockerignore and gitignore updated
Renaming of the docker build task
How to review this PR: