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

EPIC: Multiple files #570

Merged
merged 100 commits into from
Dec 29, 2020
Merged

EPIC: Multiple files #570

merged 100 commits into from
Dec 29, 2020

Conversation

munen
Copy link
Collaborator

@munen munen commented Nov 14, 2020

This epic includes multiple PRs that are only partially reviewed:

Open functional issues:

Things to write tests for:

  • (not) displaying things in Agenda, Search, Task-List with and without file settings
  • jumping to a header in another file from Agenda, Search, Task-List
  • capturing to another file with various capture template settings
  • refiling to another file with various file settings

List of things to document after before/soon after merging:

  • General functionality and awesomeness of this change
    • Add to documentation that results are limited to 200 headers
  • UX for developers: How to cope with slowing Redux (EPIC: Multiple files #570 (comment))

Follow Up Issues:

@munen munen changed the title EPIC: Multiple files [WIP] EPIC: Multiple files Nov 14, 2020
@munen
Copy link
Collaborator Author

munen commented Nov 14, 2020

@tarnung FYI, I've opened this PR to keep track of all the PRs that we've partially discussed and that are merged to develop so far.

At some point, we will have to think about properly reviewing, testing and merging your awesome changes to master. Since the PRs are (necessarily) quite big, I would offer that we do this in a pairing session. We could either do it remotely, or if you're up to it, you could come over for a day or a weekend. With the latter, we can also have a chat over coffee or tea or take a hike in the mountains of Glarus together. We don't have to make plans already, I just want to give options in case you're interested in taking them.

@munen
Copy link
Collaborator Author

munen commented Dec 6, 2020

Happy to hear you're still going strong on this feature. I'm also stoked to be working with this feature set on production, soon. Apart from the small quirks, it's been really solid on Staging and a huge improvement in how to use organice.

Cool bonus feature that you reload the sample! That was actually a bit of pain to use before - any changes to the 'doc/tutorial/sample' needed a restart to check them out. Great effort!

sync on becoming visible currently only syncs dirty files. This is not the expected behaviour. But then I don't know what is.

It's a good question. So, before it synced just the currently loaded file, independend of it being dirty. The rationale is that you might have been away for a while (think mobile app in the background for the day), you or someone else made changes to the file, you open up the app, and organice ensures that you're working with a fresh state.

With that rationale and the new capabilities, I think you're spot with reloading the loadOnStartup files 👍 From a usability perspective, it's the same as loading the app initially. There's just the technical difference that organice itself is already cached.

@munen
Copy link
Collaborator Author

munen commented Dec 7, 2020

sample works for me on Staging, now.

The changelog throws a new error, though:

image

@tarnung
Copy link
Collaborator

tarnung commented Dec 7, 2020

I fixed access to the changelog and sample from places other than the header bar.

I recently changed accessing a file in mapStateToProps in Orgfile to produce an empty default value instead of null when there is no file present. This might have been a poor decision since it has the following consequences:

  • Until a file is loaded the 'could not parse' message is shown
  • There is no way of telling if there was a parsing error (well, there is - if the parsing error message does not go away)

The offending changes are in the fix loading static files commit. I might have to undo / rethink them.
I guess we eventually should be able to handle empty files, so using an empty file as a stand in is bad anyway.

@munen
Copy link
Collaborator Author

munen commented Dec 7, 2020

The changelog does load from the settings screen now 👍

Loading the sample from settings does crash now:

image

Loading the sample from the header bar works, but it doesn't change the route:

image

@tarnung
Copy link
Collaborator

tarnung commented Dec 7, 2020

Loading the sample from settings does crash now:

Same cause as changelog. Just didn't catch it yet.

Loading the sample from the header bar works, but it doesn't change the route:

It never did.

@munen
Copy link
Collaborator Author

munen commented Dec 18, 2020

Well, this looks very good. 'sync on becoming visible' as well as loading 'sample' and 'changelog' work for me now, too.

This likely just needs a final push to get ready - getting CI green, again, and having a code review. How about we do these things in a pairing session (over Zoom)? Today, my vacation is supposed to start and should go until the 3rd of January. So, apart from the following days, I'm quite flexible: Today, 24, 28, 29, 31, 01. If any of the days in between work for you, let me know with a time proposal and then let's get this awesome PR ready and merged 🎆

@tarnung
Copy link
Collaborator

tarnung commented Dec 19, 2020

@munen I'm usually a bit low energy during the short days this season, so I didn't work on anything much the last few days.
I used staging for a while now and I agree that it's probably in a place where we can wrap it up and merge it. There's always the option to clean up more stuff later (like the sync notifications).
Let's try a pairing session. I propose the 27th in the afternoon. How about 14:00?

@munen
Copy link
Collaborator Author

munen commented Dec 23, 2020

@tarnung No worries whatsoever. First, this is a FOSS project so it's perfectly reasonable to only work on it when you're actually having fun or want to accomplish something that you're going to use yourself. Secondly, you've been productive like crazy lately(; Please, enjoy a well deserved time of cozy recovery!

Apologies for not having answered so far. I've been sick and this is actually the first of the many stacked up messages I'm replying to on the first time on the computer, again^^

27th at 14:00 sounds good. I'll send you a Zoom link via email.

Looking forward to the pairing session! All the best until then.

sample.org Outdated Show resolved Hide resolved
sample.org Outdated Show resolved Hide resolved
src/reducers/org.js Outdated Show resolved Hide resolved
     This flag didn't work so far. Also, for now we don't see a whole
     lot of value to have different settings work on all devices vs.
     on per device level. If somebody wants to build a setting in the
     future with that distinction, she can implement it better than
     the one we deleted.
@munen munen mentioned this pull request Dec 29, 2020
4 tasks
@tarnung tarnung merged commit 2163479 into master Dec 29, 2020
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