-
Notifications
You must be signed in to change notification settings - Fork 7
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
prefetch_mode implementation #68
Conversation
5b34b0c
to
a6ba1d2
Compare
Hello @seroperson, I'm struggling a bit to have some quality time to check properly your proposal. Anyway, having a quick look I like the work you did but I have two thoughts:
I'll let you know more when I could test it and play with it. By the way, thank you very much for your contribution. It is much appreciated ❤️. |
Hello! I will add tests soon, just missed their existence at all. P.S. |
Hello @seroperson , Before adding a new feature like this one I'd like give it a try to the cache option. I've been putting it off until you suggested the prefetch feature. Then I remember about the vcr gem and how easy can be setup. Here you can find the PR with the changes for the cache. Please, be free to test it in your project: gem 'jekyll-notion', git: 'https://github.com/emoriarty/jekyll-notion.git', branch: 'feat_cache-notion' What I like about this change is that little code is modified. So less chance of introducing errors and less maintenance 😉. Another highlight is that each notion request is stored in a specific file with the name of the notion resource ID, so when you want to clear the cache for a file, just look for that notion ID in the folder cache and remove the file. For the CI, you can keep the cache folder as long as you want. Or if you prefer, you can store it in your repo. Let me know if it fits for you. Cheers 🍻 ! |
Hello! I think it is good thing to add but still it is not the plain independent .md files inside your repo 🥲 With plain .md files I can edit them without notion and without this plugin at all. With cache files I can't do anything, they just make the build faster. |
Well, I don't think notion is going to go down tomorrow. So, by now you can still keep editing your files in notion or keep using your fork to download the document and edit them locally if it is what you prefer. An the end, it is a matter of preference. Me, I prefer to edit the documents in Notion than in markdown files. Therefore, I'm not going to prioritize this feature by now. I think it is more interesting having a proper cache. And right after, I'm going to refactor the test suite to use the vcr gem instead to be able to update the fixture documents easily. And possibly rework or kill the That said, I think the prefetch feature is still interesting, but it requires a little more work to be done on it. For example, to make it much more convenient, it should not be necessary to add the So what I'm proposing is to keep working on it and eventually the feature will be ready for release. Thanks again for your work and your time 🤟 and I insist, your work is much appreciated. |
Yea, I agree that it is more like personal preferences. Both approaches have their own pros and cons.
I will revise PR a little bit later. If you have some wishes/advices about this feature, feel free to mention them, I will check. |
a6ba1d2
to
f460d83
Compare
f460d83
to
cce8a61
Compare
Hello @emoriarty! I added some tests, rebased the code and renamed command to |
Hello! Recently I also managed to solve problem with short-living Notion URLs for images which were uploaded directly via UI. I'm unsure about how to deal with it using default approach, but with git-based it works perfectly. I also written a post about it: https://seroperson.me/2023/11/16/notion-jekyll-images-synchronization/ 🙂 |
Hello @seroperson, the first thing is thanking you for your contribution. Unfortunately, I've been having second thoughts regarding this feature. But first, let's see what I think what is missed in the current PR:
Anyway, these two points are of little importance. They can be tackled with agreement and a bit of code. My major concern regarding this feature is not being able to keep the original document as the source of truth. According to the current changes, executing the next if file_exists?(make_path(page)) This means that, once the documents are downloaded, this function considers any local file as the new source of truth. I wonder why anyone would want to create a document in Notion just to download it once. Subsequent changes must be made locally. Therefore, it doesn't make much sense to use Notion to create blog posts. Any changes to an old post in Notion will never be taken into account. Unless the document is deleted locally. But in case there have been local changes, they will be lost. Now let's assume the opposite. Notion is the main source of truth. What is the difference between using this function and the current caching mechanism? Any local changes will be overwritten at each execution of If the main concern of this feature is availability, you can always keep the cache in your repository. Every build in the CI/CD will not depend on Notion. But if you just want to download the documents from Notion and be able to manipulate them at will, then it seems more pertinent to create a new command based on this plugin. Or, even download the document page as Markdown from Notion using the Export option. In conclusion, I don't think this feature belongs here. The main purpose of jekyll-notion is to use Notion as a kind of CRM. Which means that Notion should remain the only source of truth. And this feature does not guarantee this purpose. Anyway, if you consider creating a new tool based in jekyll-notion, I'll be glad to link it here with a little description for those who prefers to download the files instead of using Notion as a CRM. Again, thanks for your contribution. It's really appreciated ❤️. It has made me think deeply in the vision of jekyll-notion. Let me know your thoughts. |
Hello! Thank you for your detailed response!
🫡 |
Hello @seroperson, I've found that there's already a gem that allows to download notion pages as markdown documents: jekyll-notion-import. |
Hello @emoriarty! I have created the new plugin, jekyll-fetch-notion, which is a fork of this project, but with this different kind of synchronization. I have changed some things, removed some features as for now, but in general much code is the same. I also left all the copyright things and links to the original project, so people can find the original author 🙂 Regarding |
I'm closing this PR because this feature resides in its own repository: jekyll-fetch-notion. |
Hello! Many thanks for your plugin. I want to use it to establish sync between my notion and jekyll but I'm really missing feature implemented within this PR.
Default behavior mostly works ok but it also has some pitfalls such as depending on Notion availablility, API access and internet availability. With
prefetch_mode
you can setup the sync so all your content will be stored under the git control.To setup my sync, I'm planning on scheduling just one more CI job which will fetch changes from Notion and
git commit
/git push
if any.