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

Disable current workflows for builder #4287

Conversation

jpkrohling
Copy link
Member

This PR is in preparation for moving the builder to the core repository (#4275 and #4286)

Signed-off-by: Juraci Paixão Kröhling [email protected]

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@jpkrohling
Copy link
Member Author

In addition to this PR, I will need the currently required checks to not be required anymore, such as:

image

Once the move is complete, we can enable some/most of them back.

@codecov
Copy link

codecov bot commented Oct 28, 2021

Codecov Report

Merging #4287 (34d377d) into main (db6d31e) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4287      +/-   ##
==========================================
+ Coverage   88.39%   88.45%   +0.05%     
==========================================
  Files         173      173              
  Lines       10446    10443       -3     
==========================================
+ Hits         9234     9237       +3     
+ Misses        975      969       -6     
  Partials      237      237              
Impacted Files Coverage Δ
processor/batchprocessor/splitmetrics.go 71.20% <0.00%> (+4.01%) ⬆️

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 db6d31e...34d377d. Read the comment docs.

@@ -7,6 +7,8 @@
name: "Inform Incompatible PRs"
on:
pull_request:
paths-ignore:
- 'builder/**'
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in the other PR, I would prefer cmd/builder since it is a command :)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the first step of a multi-step move. The other PR is mostly the whole repo of builder with its history, all places in one directory. If you prefer the whole application to be within cmd/builder, that's doable, but if you mean just the main.go for the application, then I will tackle that in subsequent PRs.

Comment on lines +6 to +7
paths-ignore:
- 'builder/**'
Copy link
Member

Choose a reason for hiding this comment

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

we should not ignore this.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the tidier, shouldn't be critical for this first step. Or is it?

Comment on lines 5 to 9
paths-ignore:
- 'builder/**'
pull_request:
paths-ignore:
- 'builder/**'
Copy link
Member

Choose a reason for hiding this comment

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

codeql should run all the time even for builder.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree, and I think this might already work out of the box. If it doesn't after merging this, I'll open another PR disabling CodeQL temporarily.

Comment on lines +5 to +9
paths-ignore:
- 'builder/**'
pull_request:
paths-ignore:
- 'builder/**'
Copy link
Member

Choose a reason for hiding this comment

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

links check is ok to run for the builder as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are currently failing with the link checks for the release instructions, as I reference a non-existing tag as example. I have a first ready for this, to be sent on a third PR.

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@bogdandrutu bogdandrutu merged commit 72e4e3f into open-telemetry:main Oct 28, 2021
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.

2 participants