slack-go / slack

Slack API in Go, originally by @nlopes; Maintainers needed, contact @parsley42
https://pkg.go.dev/github.com/slack-go/slack
BSD 2-Clause "Simplified" License
4.7k stars 1.14k forks source link

Implement PublishViewContextV2 to keep hash optional #1322

Open ishitamundhra opened 2 months ago

ishitamundhra commented 2 months ago
Pull Request Guidelines

These are recommendations for pull requests. They are strictly guidelines to help manage expectations.

PR preparation

Run make pr-prep from the root of the repository to run formatting, linting and tests.

Should this be an issue instead
API changes

Since API changes have to be maintained they undergo a more detailed review and are more likely to require changes.

Examples of API changes that do not meet guidelines:
lorenzoaiello commented 1 month ago

@ishitamundhra , thanks for opening the PR!

I appreciate you wanting to make this backwards compatible, but I'd like to try and limit additional functions for this and instead bite the bullet once and transition the function to use an input struct instead so that additional parameters can be added in the future without breaking backwards compatibility. How would you feel about going down that path for this PR?

ishitamundhra commented 1 month ago

@ishitamundhra , thanks for opening the PR!

I appreciate you wanting to make this backwards compatible, but I'd like to try and limit additional functions for this and instead bite the bullet once and transition the function to use an input struct instead so that additional parameters can be added in the future without breaking backwards compatibility. How would you feel about going down that path for this PR?

Hey @lorenzoaiello. Makes complete sense. I've made the changes to the current diff—can you take a look and let me know if it's in line with what you had in mind?

lorenzoaiello commented 1 month ago

@ishitamundhra , apologies for any ambiguity in my previous post. Thank you for adding the input struct, I was actually proposing not creating a V2 function but creating the breaking change on the current function to transition to an input struct. Does that make sense?

ishitamundhra commented 2 weeks ago

@ishitamundhra , apologies for any ambiguity in my previous post. Thank you for adding the input struct, I was actually proposing not creating a V2 function but creating the breaking change on the current function to transition to an input struct. Does that make sense?

Hey @lorenzoaiello, thanks for the feedbacak. Was hesistant to make the breaking change in the first place. Have updated the diff based on your comment, let me know if this feels good.