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

Remove Azure Pipelines #2807

Merged
merged 1 commit into from Feb 13, 2022
Merged

Remove Azure Pipelines #2807

merged 1 commit into from Feb 13, 2022

Conversation

mathiasvr
Copy link
Contributor

This removes the Azure Pipelines configuration, as GitHub Actions will now be used instead.

Before merging we should:

  • Update the branch protection settings for pull requests to require the GitHub Action steps instead of the Azure Pipelines steps
  • Add environment secret named REPO_TOKEN with a token having (at least) public_repo privileges
  • Confirm that nightly builds works

@mathiasvr
Copy link
Contributor Author

I changed the nightly build release step to fail the job on errors. Before, I considered the main task of the job that builds the master branch to be testing and building the branch, but now the naming suggests it is to release a nightly build, so I think it makes sense to report a failed release.

@sonarcloud
Copy link

sonarcloud bot commented Feb 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@blckmn
Copy link
Member

blckmn commented Feb 12, 2022

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> FAIL
  • cooling off period lapsed -> FAIL
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> FAIL
  • Tested label found -> FAIL
  • assigned to an approver -> FAIL
  • approver count at least three -> FAIL

Copy link
Member

@McGiverGim McGiverGim left a comment

Choose a reason for hiding this comment

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

It's ok to me 😉
@blckmn will you make the configurations to the repo?

@blckmn blckmn merged commit 85be87f into betaflight:master Feb 13, 2022
@McGiverGim
Copy link
Member

@mathiasvr I've seen that until now we prepend the date to the tag in the release. Can we continue doing in this way? Now this nightly release is lost under a lot of older releases...

@McGiverGim
Copy link
Member

McGiverGim commented Feb 13, 2022

I think is here:

tag_name: build-${{ github.run_number }}

The "correct" tag will be: "vYYYYMMYY.#", like this:

image

I'm out of the computer at this moment, but until we fix this, the release is at bottom in the nightlies.

EDIT: here is a way to do it: https://stackoverflow.com/questions/60942067/get-current-date-and-time-in-github-workflows
I'm out of the computer so I can't fix it for myself.

@mathiasvr
Copy link
Contributor Author

@McGiverGim alternatively if someone can push a new empty commit to the nightlies repo, the new tags will be linked to that and should show up in order. Both the annotated tag and release has a date, so I thought we don't also need it in the name?

@McGiverGim
Copy link
Member

I think is useful to know the date of a nightlies. The tags are ordered by tag, I doubt an empty commit will change anything.

@mathiasvr
Copy link
Contributor Author

Yes the date is useful but as mentioned it's already displayed in the release. Are you willing to try adding the commit or should I change the tag name? I think tagging a new commit to separate the azure builds from the GitHub Actions can be useful either way.

@McGiverGim
Copy link
Member

I think we need to change the tag name in the action to use the same format that until now.

@McGiverGim
Copy link
Member

Another thing, it seems the changes are empty?

https://github.com/betaflight/betaflight-configurator-nightlies/releases/tag/build-49

@mathiasvr
Copy link
Contributor Author

Ah good catch, I forgot to change that back when I changed the event from workflow run to push.

I really hope we can try adding a new commit and try this tag naming since I think just basing it on build numbers is nice and is similar to how the nightlies on Jenkins are referenced. Also as mentioned the proper date timestamps to use by API should be the ones from tag or release, not the one hard coded into the name. If it's just for consistency that's fine but not the best argument for me since I obviously think the current approach is wrong. My best guess is that it was used as a hack to order releases. I will try to experiment more with it to see why it wouldn't work like this.

@mathiasvr mathiasvr deleted the remove-azure branch February 13, 2022 15:15
@McGiverGim
Copy link
Member

The biggest problem is that the releases are ordered by name and not by date: https://github.com/betaflight/betaflight-configurator-nightlies/releases

Try you find yours here 😁

@mathiasvr
Copy link
Contributor Author

If you use the GitHub app it's the top one lol. But yes I know it's different on web, however I think a new commit will fix this and begin and order by name from that point will still work with incremental build numbers.

@McGiverGim
Copy link
Member

I don't understand why adding and empty commit will start changing the order that GitHub uses to show the releases. But we will see what happens when something more is merged.

@mathiasvr
Copy link
Contributor Author

mathiasvr commented Feb 13, 2022

Ah I just experimented with it, and now I see the order does not change because semantic versioning takes precedence over even commit dates, and since the tags prepends a v this makes them order first. However a date timestamp is not really a semantic major version, another common tag pattern would be to use the configurator version and build number such as v10.7.2-1234, but this is also not possible when e.g. v20220212 exists which is a much larger version number. Anyway, I guess I will have to add this date timestamp back then since otherwise we have to delete the existing tags.

Though now might be a good time to change it if we want to, but I can understand if this is undesirable. I still think it will be preferable down the line to have proper tagging, there is a reason an annotated tag has a complete date timestamp already.

@McGiverGim
Copy link
Member

I like too the configurator version and a build number. But as you say we need to put something that goes later than the letter v to be sure that it will go in order.
I will think a little over it. But if you have time, please open the PR with the fix in the changes and we can discuss there all of this. This PR is closed and people usually does not see it

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

Successfully merging this pull request may close these issues.

None yet

4 participants