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

changing circleci workflow name #2887

Merged

Conversation

algojack
Copy link
Contributor

Summary

Fixing naming due to new way of displaying CircleCI workflow

Test Plan

Check tests at the bottom of Conversation tab and on the Checks tab

@onetechnical
Copy link
Contributor

This requires special coordination with the github checks to pass (or an override).

@algobarb
Copy link
Contributor

I think I would rather not use spaces if we can help it in the yaml file. Maybe something like circleci-build-and-tests workflow?

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021

Codecov Report

Merging #2887 (f917d15) into master (b44e3d9) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2887   +/-   ##
=======================================
  Coverage   47.10%   47.10%           
=======================================
  Files         349      349           
  Lines       56430    56430           
=======================================
+ Hits        26580    26583    +3     
+ Misses      26864    26863    -1     
+ Partials     2986     2984    -2     
Impacted Files Coverage Δ
network/wsPeer.go 74.37% <0.00%> (-0.28%) ⬇️
catchup/service.go 69.35% <0.00%> (ø)
util/metrics/counter.go 80.95% <0.00%> (+2.38%) ⬆️
util/metrics/gauge.go 70.66% <0.00%> (+2.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b44e3d9...f917d15. Read the comment docs.

@algojack
Copy link
Contributor Author

I think I would rather not use spaces if we can help it in the yaml file. Maybe something like circleci-build-and-tests workflow?

May I ask why? It specifically shows names like that in their example here. And it is much more human readable than using dashes or underscores.

@algobarb
Copy link
Contributor

I think I would rather not use spaces if we can help it in the yaml file. Maybe something like circleci-build-and-tests workflow?

May I ask why? It specifically shows names like that in their example here. And it is much more human readable than using dashes or underscores.

I think the reason to avoid spaces in the keys is so it's closer to usual YAML convention where keys can easily be referenced, if the need arises. Also, most of the examples they have don't use spaces for workflow names: https://circleci.com/docs/2.0/sample-config/

@algojack
Copy link
Contributor Author

algojack commented Sep 14, 2021

@algobarb good point, updated to one of their examples "build_and_test" (also we already know it's CircleCI since it is wrapped in title "CircleCI Checks" at least in the Checks tab... let me know if you think we should bring back circleci piece)

@algobarb
Copy link
Contributor

@algobarb good point, updated to one of their examples "build_and_test" (also we already know it's CircleCI since it is wrapped in title "CircleCI Checks" at least in the Checks tab... let me know if you think we should bring back circleci piece)

I don't think explicitly saying "circleci" is necessary.

@algojack
Copy link
Contributor Author

this PR can be merged whenever we are ready. The build_pr job is not going to finish, since it was renamde to build_and_test by this PR. We do need to change the required jobs setting to have the new name build_and_test instead of build_pr when we merge this.

@yaovi-a yaovi-a assigned algojohnlee and unassigned algojack Oct 14, 2021
@algojohnlee algojohnlee merged commit bcd77ee into algorand:master Oct 18, 2021
@algojack algojack deleted the Changing-circleci-build-integration-name branch October 18, 2021 15:11
cce pushed a commit to cce/go-algorand that referenced this pull request Oct 28, 2021
Fixing naming due to new way of displaying CircleCI workflow.
@egieseke egieseke mentioned this pull request Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants