Skip to content

Added devconfig-validator and comment handler #3816

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

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

boblodgett
Copy link
Contributor

Description

The DevConfig Validator is a GitHub workflow that automatically checks if a DevConfig file is needed for your PR and helps you create one if necessary. The workflow detects changes to core and service files and determines if a DevConfig file is required.

Workflow

  1. When you create or update a PR, the validator checks if a DevConfig file exists.
  2. If no DevConfig file is found but changes require one, the validator adds a comment to your PR with a preview of the suggested DevConfig.
  3. You can then add the DevConfig file using one of the provided commands.

Commands

The DevConfig Validator supports the following commands that you can use in PR comments:

  • /add-devconfig: Adds the preview DevConfig file to your PR. You can also provide your own JSON to customize the DevConfig.
  • /delete-devconfig: Removes the DevConfig file from your PR.
  • /amend-devconfig: Updates the existing DevConfig file with new JSON content.

Example usage:

/add-devconfig
```json
{
  "core": {
    "changeLogMessages": [
      "Your custom changelog message"
    ],
    "type": "patch",
    "updateMinimum": true
  }
}
```

Test PR showing interaction: https://github.com/aws/aws-sdk-net-staging-dev/pull/1

Associated PR for the offline tool referenced by the validator: #3811

TODO:

  • Need to verify how this works with forks
  • Should exclude *.sln files from the auto service discovery checks.
  • Possibly better initial changelog message creation.
  • May not want to trigger the initial validator on edit of a PR. It could be manually retrigged instead.

Motivation and Context

Need a way for developers (Internal/External) to know devconfig files are needed and to give the best starting point for getting those devconfig files added easily to their PRs.

Testing

Test PR showing interaction: https://github.com/aws/aws-sdk-net-staging-dev/pull/1

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty self-requested a review May 15, 2025 16:47
@afroz429 afroz429 self-requested a review May 15, 2025 16:56
```

- `changeLogMessages`: (Required) An array of messages to include in the changelog under the core section.
- `type`: (Required) The part of the version string to increment. Valid values are `minor` and `patch`.
Copy link
Member

Choose a reason for hiding this comment

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

You might want to give guidance on when to use patch versus minor.


- `changeLogMessages`: (Required) An array of messages to include in the changelog under the core section.
- `type`: (Required) The part of the version string to increment. Valid values are `minor` and `patch`.
- `updateMinimum`: (Required) Whether to update the minimum core version for all services.
Copy link
Member

Choose a reason for hiding this comment

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

Give an example of when they might want to set this to true.

You can use the provided script to generate a DevConfig file:

```
./buildtools/add-devconfig.bat
Copy link
Member

Choose a reason for hiding this comment

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

Call out the bat file for windows or ./buildtools/add-devconfig.sh for mac/linux

echo "No DevConfig files found in this PR"
fi

- name: Check if DevConfig is needed
Copy link
Member

Choose a reason for hiding this comment

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

I might be missing something but depending on when the contributor branched from development there might be dev config files for other unshipped PRs in the forked branch. Would the workflow say all is good because there is a dev config in the folder even though it is unrelated to what the PR does?

@@ -0,0 +1,219 @@
name: DevConfig Validator
Copy link
Member

Choose a reason for hiding this comment

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

Should this workflow be skipped if the target is anything other then development or the V3 development? What I'm thinking is the PR is an incremental work for a feature branch a dev config is not required.

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.

3 participants