BigSamu / OpenSearch_Changelog_Workflow

A reusable workflow for automating changelog and release notes generation processes for OpenSearch repos
Apache License 2.0
0 stars 4 forks source link

Feat/manual changesets #61

Closed BigSamu closed 4 months ago

BigSamu commented 4 months ago

Part-1 of manual approach -> allow contributors and maintainers to optionally install the Github App and create the changesets manually

Pending Part-2 of manual approach -> enforce that changesets manually created by a user follow the rules defined

Other updates:

Refactoring of main.js done for better readability

BigSamu commented 4 months ago

I Like the refactor. Its so much easier to read. A few overall comments:

  1. Try not to combine multiple changes in a single PR makes it harder to understand which change is relevant to the actual feature.

You right! It is my mistake. But for this specific case, it is sort of annoying to do the testing in Github itself or add commit messages for every atomic change. I would like your feedback on this. Feeling that I loose too much time creating atomic commits. But you have indeed an important point!

  1. Splitting the main file would have been nice, but i like the code splitting so far.

Just finish this. Took me a while. Now is much cleaner.

  1. Nice job on making sure that there are tests on this!

This will take tons! I have a friend that wants to collaborate on OpenSearch. I told him best way is to work with me on this feature. I will give you more updates after talking with him.

ashwin-pc commented 4 months ago

You right! It is my mistake. But for this specific case, it is sort of annoying to do the testing in Github itself or add commit messages for every atomic change. I would like your feedback on this. Feeling that I loose too much time creating atomic commits. But you have indeed an important point!

You dont need to make atomic commits, just splitting the refactor out into its own PR would be sufficient

BigSamu commented 4 months ago

You right! It is my mistake. But for this specific case, it is sort of annoying to do the testing in Github itself or add commit messages for every atomic change. I would like your feedback on this. Feeling that I loose too much time creating atomic commits. But you have indeed an important point!

You dont need to make atomic commits, just splitting the refactor out into its own PR would be sufficient

Mmmm I see. So you mean that the best practice is to work on each new feature or refactor in a separate PR, correct?

BigSamu commented 4 months ago

This looks good to me! Lets merge it!

Perfect. Continuing to merge!