Skip to content
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

Feature: note.rs #16

Merged
merged 4 commits into from
Jan 28, 2024
Merged

Feature: note.rs #16

merged 4 commits into from
Jan 28, 2024

Conversation

falk-werner
Copy link
Owner

This PR introduces a new module: note.rs (finally)
There is no new functionality, just moved the current implementation from main to the new module.

@falk-werner falk-werner enabled auto-merge (rebase) January 27, 2024 19:45
@fersaru
Copy link
Collaborator

fersaru commented Jan 27, 2024

One Note, two notes, all a part of a notebook.
I would like that

@falk-werner falk-werner disabled auto-merge January 27, 2024 21:13
@falk-werner
Copy link
Owner Author

One Note, two notes, all a part of a notebook. I would like that

I renamed the struct to Notebook, just to show how this would look like.
Note that this would lead also to renaming of some of the APIs, as notebook.list() does not make any sense, since there is only one. It is also very common to use singular in APIs (see swagger's petstore) and also in database tables. (I took a look at swaggerhub and also found quite a number of APIs that uses plural, but it seems to me, that notes would be preferred by them.)

Also note, that I've only applied this change to the backend. When applied to the frontend, it would have impact on note.js, since we plan to share the same frontend in the future.

@fersaru
Copy link
Collaborator

fersaru commented Jan 28, 2024

I don't like the idea of changing the frontend so it becomes unique to this note.* implementation.
The "interface" should present the same function signatures as the other backend implementations.
But how the function is implemented should be backend specific.

In the current naming I don't like the _ in list_notes because when I think in objects it reads like there should be a notes object.
In objects speaking I thought one notebook has all the configs it needs, it holds a quantity of notes and has the functionalities to present them in a desired format (like a list of only the names). A note would then have knowledge about it's name, it's path and it's tags.

But I think this becomes too complex for the initial idea of having a simple note-tool.
I didn't aim for a discussion about a change in the API with my proposal. I was just thinking "out loud".

I think going back to the "note" naming is closest to the note.* idea, so i would go back to it for now.

@falk-werner falk-werner enabled auto-merge January 28, 2024 09:52
@falk-werner falk-werner merged commit 1c0dec6 into main Jan 28, 2024
5 checks passed
@falk-werner falk-werner deleted the feature/note.rs branch January 28, 2024 09:59
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.

2 participants