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

Add workflow for changelog verification #4085

Merged
merged 7 commits into from
Aug 23, 2022

Conversation

kotwanikunal
Copy link
Member

@kotwanikunal kotwanikunal commented Aug 2, 2022

Signed-off-by: Kunal Kotwani kkotwani@amazon.com

Description

  • The changes under this PR build on adding a changelog enforcer using the GHA Changelog Enforcer following the Keep A Changelog format
  • What this solves for currently:
    • Enforcing all new PRs made to the repo have a changelog attached to them
    • Dependabot PRs have the appropriate changes automatically added to the CHANGELOG.md file
  • What this currently does not solve for:
    • Backported PRs will have issues since the changes will not be automatically added to the older version within the changelog file

Issues Resolved

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

@kotwanikunal
Copy link
Member Author

> Task :libs:opensearch-grok:compileTestJava
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

Problems handling incoming cache access requests.
java.lang.OutOfMemoryError: Java heap space

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Gradle Check (Jenkins) Run Completed with:

@tlfeng tlfeng added CI CI related v3.0.0 Issues and PRs related to version 3.0.0 labels Aug 4, 2022
@kotwanikunal
Copy link
Member Author

@dblock What do you think of this?
Related: #1868

name: "Changelog Verifier"
on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review, labeled, unlabeled]
Copy link
Member

Choose a reason for hiding this comment

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

Why not all PRs?

Copy link
Member Author

Choose a reason for hiding this comment

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

The default mechanism for pull_request trigger is as follows:

By default, a workflow only runs when a pull_request event's activity type is opened, synchronize, or reopened.

I have updated the GHA config to reflect all the types I found could match our case.

CHANGELOG.md Outdated

## [ UNRELEASED ]
### Added
- Github workflow for changelog verification
Copy link
Member

Choose a reason for hiding this comment

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

Let's start with a format that includes a link to the PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

The only concern I have with that is it will become a 2 step process -

  1. Add changelog with dummy link and raise the PR
  2. Update the changelog and commit again with the PR link

Let me check if there is an automated mechanism/property within the used GHA to achieve this.

Copy link
Member

Choose a reason for hiding this comment

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

Working backwards from the end goal of users being able to find what the change was I think that's an acceptable overhead.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I love it. Super simple. Let's update developer docs too?

@kotwanikunal
Copy link
Member Author

I love it. Super simple. Let's update developer docs too?

Yes. I will update the PR as soon as we have the syntax and process finalized.

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

i like the idea of being able to enforce this in an automated way so that it isn't forgotten!

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated
## [ UNRELEASED ]
### Added
- ...
- Add support for FooBar client ([#9999](https://github.com/opensearch-project/OpenSearch/pull/9999))
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to change the format to include the contributor alias? I want to give people easy credit for the work they do.

Copy link
Contributor

Choose a reason for hiding this comment

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

as i already wrote in opensearch-project/security#1821 (comment) i personally don't think listing the contributors is a good idea. when reading the changelog i want to know what changed and what it means for me - any other information is just annoying clutter and will be ignored and slows me down. i can always follow the links or check the commit history to know more.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could potentially use github aliases within the format, but like the PRs, it would be a self update process.
+1 on following the PR links / browsing commit history to reduce information overload.

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@kotwanikunal
Copy link
Member Author

Gradle check:


FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':distribution:bwc:minor:buildBwcLinuxTar'.
> Building 2.2.0 didn't generate expected file /var/jenkins/workspace/gradle-check/search/distribution/bwc/minor/build/bwc/checkout-2.x/distribution/archives/linux-tar/build/distributions/opensearch-min-2.2.0-SNAPSHOT-linux-x64.tar.gz

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@kotwanikunal
Copy link
Member Author

kotwanikunal commented Aug 18, 2022

Unrelated:

Tests with failures:
 - org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests.testSortMissingLastReverse

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated

For example, as a contributor, my change adds support for a new `FooBar` client, the changes would look like -

```
Copy link
Contributor

Choose a reason for hiding this comment

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

tbh, i don't really see the value in duplicating the content of a near-empty CHANGELOG.md here again. why not just write

Suggested change
```
2. Add an entry for your change to the corresponding section in [`CHANGELOG.md`](CHANGELOG.md) and make sure that you reference the pull request there.

or, if you truly want to keep it, at least turn it into markdown syntax highlighting:

Suggested change
```
```md

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the PR!

CONTRIBUTING.md Outdated Show resolved Hide resolved
Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dblock dblock merged commit 5327767 into opensearch-project:main Aug 23, 2022
@dblock
Copy link
Member

dblock commented Aug 23, 2022

I merged it. Let's backport to 2.x and try to run with this.

@dblock dblock added the backport 2.x Backport to 2.x branch label Aug 23, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 23, 2022
* Add workflow for changelog verification

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update format for changelog, add developer documentation

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update link reference to be relative to project

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Fix links for CHANGELOG versions

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update contribution guide

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit 5327767)
kotwanikunal added a commit that referenced this pull request Aug 24, 2022
* Add workflow for changelog verification

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update format for changelog, add developer documentation

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update link reference to be relative to project

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Fix links for CHANGELOG versions

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update contribution guide

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit 5327767)

Co-authored-by: Kunal Kotwani <kkotwani@amazon.com>
@dblock
Copy link
Member

dblock commented Aug 25, 2022

@kotwanikunal Are you going to take this on as a campaign for all plugins once it's stable here? Maybe we need to open sub-issues in those repos?

@kotwanikunal
Copy link
Member Author

@dblock I can run that once we ensure the process and any further automation/guardrails are in place.
I am also backporting all the changes to 1.x right now.

opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 29, 2022
* Add workflow for changelog verification

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update format for changelog, add developer documentation

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update link reference to be relative to project

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Fix links for CHANGELOG versions

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update contribution guide

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit 5327767)
kotwanikunal added a commit that referenced this pull request Aug 29, 2022
* Add workflow for changelog verification

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update format for changelog, add developer documentation

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update link reference to be relative to project

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Fix links for CHANGELOG versions

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

* Update contribution guide

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>

Signed-off-by: Kunal Kotwani <kkotwani@amazon.com>
(cherry picked from commit 5327767)

Co-authored-by: Kunal Kotwani <kkotwani@amazon.com>
@lukas-vlcek
Copy link
Contributor

@kotwanikunal @dblock I think the changelog doc link in the checklist section is broken.

I recently opened this PR #4360 and you can see that the link points to https://github.com/opensearch-project/OpenSearch/CONTRIBUTING.md#changelog which is wrong. Correct link is https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#changelog.

Maybe we need to change the relative link from [Changelog](../CONTRIBUTING.md#changelog) to [Changelog](../blob/main/CONTRIBUTING.md#changelog) ?

@kotwanikunal
Copy link
Member Author

@kotwanikunal @dblock I think the changelog doc link in the checklist section is broken.

I recently opened this PR #4360 and you can see that the link points to https://github.com/opensearch-project/OpenSearch/CONTRIBUTING.md#changelog which is wrong. Correct link is https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#changelog.

Maybe we need to change the relative link from [Changelog](../CONTRIBUTING.md#changelog) to [Changelog](../blob/main/CONTRIBUTING.md#changelog) ?

I'll add in the fix. Thanks for bringing it to notice.

@kotwanikunal
Copy link
Member Author

@lukas-vlcek Fixed it with #4364

@kotwanikunal kotwanikunal deleted the changelog-verifier branch September 1, 2022 17:24
@dreamer-89 dreamer-89 added the v2.3.0 'Issues and PRs related to version v2.3.0' label Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.x backport 2.x Backport to 2.x branch CI CI related v2.3.0 'Issues and PRs related to version v2.3.0' v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants