-
Notifications
You must be signed in to change notification settings - Fork 45
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
Some quality of life improvements #31
Some quality of life improvements #31
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate the PR - there are few review comments which you might wanna have look at them.
|
||
namespace Notion.Client.Extensions | ||
{ | ||
public static class QualityOfLifeExtensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kurupapuru I'm sure there could be many more helpful extensions we may want to add. So I think it is better if we create a separate library (NuGet) to maintain them. So the user can inject them as they require. Let me know your views?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KoditkarVedant sorry for such late response, expected to get notification from github
I think that is a good option. I don't have any experience with creating projects for publishing nuget packages, but if you will create repository with needed structure I can make pull request there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kurupapuru I've created a repository so you can add these extensions there.
Repo: https://github.com/notion-dotnet/notion-sdk-extensions
@@ -8,5 +8,16 @@ public class DatabasesQueryParameters : IDatabaseQueryBodyParameters | |||
public List<Sort> Sorts { get; set; } | |||
public string StartCursor { get; set; } | |||
public string PageSize { get; set; } | |||
|
|||
public DatabasesQueryParameters CreateCopy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is only required by extensions then you can move it there. Apart from that we may need deep copy of an object. ref
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, just for extension
@@ -6,6 +6,7 @@ public interface INotionClient | |||
IDatabasesClient Databases { get; } | |||
IPagesClient Pages { get; } | |||
ISearchClient Search { get; } | |||
IBlocksClient Blocks { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kurupapuru if you could create a separate PR for this fix would be great so I can merge that and create a release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for catching this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw that this has already been fixed, again sorry for late response
#35
Hi @Kurupapuru, I've created a repository so you can add these extensions there. I am closing this PR. Repo: https://github.com/notion-dotnet/notion-sdk-extensions |
Added
BlocksClient
toNotionClient
, not sure why it wasn't thereAdded
GetTitle
toPage
as extensionAdded
GetAllPlainText
toTitlePropertyValue
as extensionAdded
QueryToEndAsync
toIDatabaseClient
as extensionAdded method
CreateCopy
toDatabasesQueryParameters
forQueryToEndAsync
method, not sure if that is right wayI understand that something can be questionable and I'm new to contributing so feel free to point at my mistakes