-
Notifications
You must be signed in to change notification settings - Fork 95
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
ci: add "exp" PR type #323
Conversation
8606cf7
to
0e9ab2b
Compare
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
0e9ab2b
to
c5e3b89
Compare
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
47e04f3
to
394cb5e
Compare
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.
Not language expert.. just want to raise the discussion to think more about the naming and messages.
Should the pr title be "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.
Also wondering if we really need to run the full e2e tests for any .github/**.yaml
changes. Should I also create a new ci workflow to only lint github workflow yaml, like how we do it in ci-docs?
@pendo324 The e2e test itself can be added/changed in **.yaml so it is a bit different with .doc. Even specifying a subset of yamls, it can be risky. |
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
I added |
@pendo324 This will cause changing ci-github-yaml.yaml itself will not run e2e tests, which is risky and require more manual effort to review the changes. Let's discuss in a separated issue/PR if you still want to propose this. |
I don't necessarily think that's risky, but I see how changing the "actual" e2e test ( |
Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Skipping CI / e2e-tests since this change doesn't impact functionality (just a GitHub workflow YAML change). |
Issue #, if available: *Description of changes:* - Previously we added exp to release-please.yaml in #323, but lint-pr-title has [its own configuration for custom types](/~https://github.com/amannn/action-semantic-pull-request). - Not doing this causes issue ([seen here](/~https://github.com/runfinch/finch/actions/runs/4547098114/jobs/8016553677)) with PR titles using exp *Testing done:* - N/A - [x] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Justin Alvarez <alvajus@amazon.com>
Issue #, if available:
Description of changes:
Testing done:
N/A
I've reviewed the guidance in CONTRIBUTING.md
License Acceptance
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.