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

Devops/clean up workflows #361

Merged

Conversation

danuw
Copy link
Collaborator

@danuw danuw commented Jun 5, 2023

Pull Request

Issue Number: (Link to Github Issue or Azure Dev Ops Task/Story)

Summary

Brings together the different workflows triggered by a pull request.
It mixes 6 workflows (out of 8 that be moved - 2 are for later) into 1.

Changes

  • clearer naming (workflow is numbered so easier to fit in the process)
  • new PR workflow updated
  • build and test solution once across the modified workflows
  • runs other checks triggered by a change to dev with dependencies so not all jobs of a workflow run when there is a(/are) failure(/s)

Checklist

  • Local Tests Passing?
  • CICD and Pipeline Tests Passing?

Are there API Changes?

If yes, what are the expected API Changes? Please link to an API-Comparison
workflow with the API Diff.

Is this a breaking change?

If yes, what workflow does this break?

Anything else?

Other comments, collaborators, etc.

Please follow
GitHub's suggested syntax
to link Pull Requests to Issues via keywords

This PR Closes #<issue_number>

@danuw danuw requested a review from vaughanknight as a code owner June 5, 2023 23:28
Copy link
Collaborator

@Willmish Willmish left a comment

Choose a reason for hiding this comment

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

Please fix DCO issues :)

@@ -1,7 +1,7 @@
# Docs for the Azure Web Apps Deploy action: https://github.com/Azure/webapps-deploy
# More GitHub Actions for Azure: https://github.com/Azure/actions

name: Build and deploy ASP.Net Core app to Azure Web App - carbon-aware-api
name: 2.a-Build and deploy ASP.Net Core app to Azure Web App - carbon-aware-api
Copy link
Member

Choose a reason for hiding this comment

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

Should we deploy the app from the container built by 2-pre-release.yaml ? You mentioned about this before:
#347 (comment)

can we update it to deploy from the newly deployed package (if triggered in DEV, using latest "pre" tagged image, and if from MAIN, using "latest" tagged image) please?

We can work for this in another issue of course.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we should but I was trying to at least get this through. I can see that even like that I will need to improve further (some tasks repeat but trying to keep the changes low risk so it can be reviewed more easily), but I now have dependencies so if a workflow that describes a foundation steps for others fail, it can just stop there instead of running all workflows even when the solution does not compile...

Can you help review so we can get that through and merge with your changes to get them through too please? @YaSuenag

(so deploy will be a different PR, but as you can see I already had a sample that I commented out in the end)

cc @Willmish @vaughanknight

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My main problem was DCO - had some conflicts on my local but will go back to that. In the meantime, please start reviewing

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2023

Codecov Report

Merging #361 (0f18fa9) into dev (e9885d7) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #361   +/-   ##
=======================================
  Coverage   74.21%   74.21%           
=======================================
  Files          77       77           
  Lines        2637     2637           
  Branches      266      266           
=======================================
  Hits         1957     1957           
  Misses        598      598           
  Partials       82       82           

@danuw danuw self-assigned this Jun 19, 2023
@danuw danuw added the devops Identify issues that impact delivery lifecycle as opposed to functionality (~basically code changes) label Jun 19, 2023
Copy link
Member

@YaSuenag YaSuenag left a comment

Choose a reason for hiding this comment

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

LGTM. Let's improve some topics as another issues.

@danuw danuw force-pushed the devops/clean-up-workflows branch from 71d0c12 to 0f18fa9 Compare July 3, 2023 20:35
@danuw
Copy link
Collaborator Author

danuw commented Jul 3, 2023

@Willmish - DCO completed
Let's get this through - will simplify the workflows, and we can start to update the release ones (Hosted API + packages)
thx

Copy link
Collaborator

@Willmish Willmish left a comment

Choose a reason for hiding this comment

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

LGTM

@Willmish Willmish merged commit 5caa384 into Green-Software-Foundation:dev Jul 11, 2023
romanlutz pushed a commit to romanlutz/carbon-aware-sdk that referenced this pull request Jul 27, 2023
* cleaning workflows

Signed-off-by: Dan Benitah <[email protected]>

* pull request should on trigger for pull request event - removing on push for PRs

Signed-off-by: Dan Benitah <[email protected]>

* syntax

Signed-off-by: Dan Benitah <[email protected]>

* syntax

Signed-off-by: Dan Benitah <[email protected]>

* moving markdown linting and disabling 2.a-deploy

Signed-off-by: Dan Benitah <[email protected]>

* adding some jobs dependencies so packaging does not run if code does not build etc...

Signed-off-by: Dan Benitah <[email protected]>

* moving code analysis into the build step and clean up

Signed-off-by: Dan Benitah <[email protected]>

---------

Signed-off-by: Dan Benitah <[email protected]>
Co-authored-by: danuw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops Identify issues that impact delivery lifecycle as opposed to functionality (~basically code changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants