-
Notifications
You must be signed in to change notification settings - Fork 45
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.
This is pretty much complete, let me know if there is any other thoughts/suggestions.
.github/workflows/ci.yml
Outdated
pull_request: | ||
branches: [main] | ||
# TODO(meain): update references to release-ci before merge | ||
branches: [ release-ci ] |
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.
Keeping it as release-ci
so that the jobs will be run for "dry run". Once the changes look good, we can change this to main
before merge.
.github/workflows/ci.yml
Outdated
precheck: | ||
uses: alcionai/corso/.github/workflows/_filechange_checker.yml@main | ||
Precheck: | ||
uses: alcionai/corso/.github/workflows/_filechange_checker.yml@release-ci # TODO(meain): post-merge of docs-ci: change this to main |
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.
We cannot reference an action without a tag, will change this to main as well before we do a merge. Keeping it like this so that the run will be successful.
.github/workflows/ci.yml
Outdated
uses: ./.github/actions/go-setup-cache | ||
if: startsWith(github.ref, 'refs/tags/') || github.ref == 'refs/heads/release-ci' || needs.precheck.outputs.docfileschanged == 'true' |
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.
startsWith(github.ref, 'refs/tags/')
will be true for releases
github.ref == 'refs/heads/main
will be true for commits to main
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.
The fully-formed ref of the branch or tag that triggered the workflow run. For workflows triggered by push, this is the branch or tag ref that was pushed. For workflows triggered by pull_request, this is the pull request merge branch. For workflows triggered by release, this is the release tag created. For other triggers, this is the branch or tag ref that triggered the workflow run. This is only set if a branch or tag is available for the event type. The ref given is fully-formed, meaning that for branches the format is refs/heads/<branch_name>, for pull requests it is refs/pull/<pr_number>/merge, and for tags it is refs/tags/<tag_name>. For example, refs/heads/feature-branch-1.
mv ./src/cmd/mdgen/cli_markdown/* ./docs/docs/cli/ | ||
rm -R ./src/cmd/mdgen/cli_markdown/ | ||
|
||
- uses: actions/upload-artifact@master |
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.
This is done as we need this data in the publish-docs job.
@@ -0,0 +1,10 @@ | |||
# Changelog |
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.
Add a CHANGELOG as discussed in #571 (comment)
@@ -9,7 +9,7 @@ const config = { | |||
title: 'Corso Documentation', | |||
tagline: 'Free, Secure, and Open-Source Backup for Microsoft 365', | |||
url: 'https://corsobackup.io', | |||
baseUrl: '/', | |||
baseUrl: process.env.CORSO_DOCS_BASEURL || '/', |
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.
Since we are currently deploy to /preview
this is used to set that.
goos: | ||
- linux | ||
- windows | ||
- darwin | ||
goarch: # other options: 386, arm | ||
- amd64 | ||
- arm64 |
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.
All the different binary versions that we are releasing now.
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.
Let's also add an ignore
section so we don't build: windows-arm64 and darwin-amd64
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.
Any reason why we are not building darwin-amd64
? All except the latest m1/m2 based macs will be using amd64
if I am not mistaken.
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.
@vkamra Did you mean darwin-arm64?
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.
Even with darwin-arm64
, I think it would be a good idea to keep it as that will make sure we can run natively on m1/m2 macs.
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 would agree.
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 was actually thinking we only build for M1+. If we see asks for others we can add.
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 feel like more than 60% developers at least will be using amd64 based machines, but if you think we should drop it, I'll add an ignore entry. The cost for use to include it would be about extra ~15m in the CI.
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.
Let's go with yours and Niraj's recommendation and keep both for darwin.
@meain - this looks right. I wanted to look at the github actions running but they seem to be stuck. |
Not sure what happened there with GH Actions. I just force pushed a timestamp change to trigger a rerun. The total run takes >1.5 hours though, mainly goreleaser taking time to build all the binaries but I guess this is to be expected as we are building {linux,mac,window}*{amd64,arm64} (6 builds) and each build on my machine with 12 cores was taking ~4min. That said, in most cases we will just be running lints which takes pretty much the same time as before, ie ~30min. |
The docs will be pushed to https://d2jrnhxpb9sco2.cloudfront.net/preview/index.html |
This should now be ready unless anyone has any comments. Once a review is complete, I'll make the changes mentioned in |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
* Setup CI for release * Initial commit of CHANGELOG.md * Pin vale and markdownlint-cli to current versions * Update branch name for CI * Pull request workflow job
## Description This sets up the metrics configs using `ldflags`. Keeping it in draft as I wanted to wait till #1052 is merged as that will affect how we use it in the CI. I have currently set the base branch to `release-ci` on GH, the branch for #1052 instead of `main` as the diff would make more sense that way. ## Type of change <!--- Please check the type of change your PR introduces: ---> - [ ] 🌻 Feature - [ ] 🐛 Bugfix - [ ] 🗺️ Documentation - [ ] 🤖 Test - [x] 💻 CI/Deployment - [ ] 🐹 Trivial/Minor ## Issue(s) <!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. --> * #1067 ## Test Plan <!-- How will this be tested prior to merging.--> - [x] 💪 Manual - [ ] ⚡ Unit test - [ ] 💚 E2E
Description
This takes care of all the CI related to #69 and #571.
TODO
release-ci
withmain
release-ci
tomain
Post merge TODO
Type of change
Issue(s)
make check
) to CI/GitHub Actions #69Test Plan