laravel-notion-api icon indicating copy to clipboard operation
laravel-notion-api copied to clipboard

Add Select::id()

Open richardhj opened this issue 2 years ago • 3 comments

Shorthand method for when you want to reference a select by its id rather than textual representation.

richardhj avatar Apr 10 '23 00:04 richardhj

PR Summary

  • Added a new method to the Select class This method, called id, takes in a parameter of type string and returns an instance of the Select class.
  • Improved selectItem object configuration The new method creates a selectItem object, sets its ID property with the provided value, and assigns it as content for the current selectProperty object.

what-the-diff[bot] avatar Apr 10 '23 00:04 what-the-diff[bot]

Thanks @richardhj for the PR!

I'd change the name to ::valueById(...) to make it more clear what is done. Just ::id(...) could be confusing, because it breaks apart from the pattern of creating new properties by ::valueSomething(...).

We'd plan to merge this with 1.1.0. Please add tests to your implementation, so we can merge with the dev branch.

johguentner avatar Apr 10 '23 07:04 johguentner

Hi @richardhj,

just wanted to check with you, if you want to write tests for this feature. Within our contribution guidlines (https://github.com/5am-code/laravel-notion-api/blob/main/CONTRIBUTING.md) you can double check what is important for a PR to this repo. We want to keep this repo as clean as possible, so we want PRs to stick to those guidelines as best as possible - aside from some exceptions.

If you need any help, please let us know.

We will push v1.1.0 around the end of the week and will cleanup all remaining PRs marked for v1.1.0.

Thank you very much!

johguentner avatar May 02 '23 11:05 johguentner