Skip to content
This repository was archived by the owner on Jun 8, 2021. It is now read-only.

Remove leading & trailing spaces before adding task - #84

Open
schmittlauch wants to merge 6 commits into
Armadill0:nextfrom
schmittlauch:trim_tasks
Open

Remove leading & trailing spaces before adding task#84
schmittlauch wants to merge 6 commits into
Armadill0:nextfrom
schmittlauch:trim_tasks

Conversation

@schmittlauch

Copy link
Copy Markdown

As the keyboard's autocorrect usually appends a space to the completed
word, sometimes already existing tasks haven't been reopened properly.
contributes to #82

This is not yet ready to be merged as I should also add a database migration which trims all existing tasks, shouldn't I?

As the keyboard's autocorrect usually appends a space to the completed
word, sometimes already existing tasks haven't been reopened properly.
contributes to Armadill0#82
@schmittlauch

Copy link
Copy Markdown
Author

Btw, is there a way to provide fake api keys for Dropbox? Currently I just modified the projectfile to make building work without Dropbox keys.

@Armadill0

Armadill0 commented Oct 28, 2016

Copy link
Copy Markdown
Owner

Thanks for the solution. There is another place on the Edit page where this has to be implemented: https://github.com/Armadill0/harbour-tasklist/blob/master/qml/pages/EditPage.qml#L117
More or less the same code should be needed there. Would be great if you could add this to the pull request. 😄

About the existing task thing: I don't want to manipulate all the data which has already been entered at once. This would take some time e.g. at the application start which would degrade the user experience, especially for the power users with many tasks. And as we would never know when all users have migrated to trimmed data, we couldn't remove the code to enhance the user experience again.

Instead I'm thinking about to compare the newly entered task with the already existing ones, but trim their value and update them afterwards with the trimmed value of leading or trailing spaces have been found. But this could possibly lead to longer waitings for new tasks.

I think I have to do some speed comparisons after this is being merged to find the best solution. 😃

Regarding your Dropbox question, have a look at the end of the environment preparation: https://github.com/Armadill0/harbour-tasklist/wiki

@schmittlauch

Copy link
Copy Markdown
Author

Instead I'm thinking about to compare the newly entered task with the already existing ones, but trim their value and update them afterwards with the trimmed value of leading or trailing spaces have been found. But this could possibly lead to longer waitings for new tasks.

I'm for making a clean cut using a migration which has only to be run once instead of adding complexity we'll never getting rid of again. Just think of how much unnecessary logic and complexity this would add 😱

And as we would never know when all users have migrated to trimmed data, we couldn't remove the code to enhance the user experience again.

What about introducing a db versioning schema in a previous update so we can just compare the schema version number? And we can also add a migration screen which shows the user why the application takes longer to start exactly this one time.
Or maybe there's the possibility to do migration in a rpm post-install script? This would be even better.

More or less the same code should be needed there

Let's see if I even can reuse the code there without repeating it.

Btw, are there any unit or integration tests?

@Armadill0

Armadill0 commented Oct 28, 2016

Copy link
Copy Markdown
Owner

Hm, not a bad idea to try it over the db versioning, which already exists because of former changes. But I don't know if it is only possible to change the db schema or also fire some update query. Have to look into this.

Currently there aren't any automated tests. Most things are tested by myself before each release and some things have been found by the Jolla QA guys/girls and a few things were found by users. 😄

- also moved trimmed() function to own module
@schmittlauch

Copy link
Copy Markdown
Author

I looked into how to do migrations, and while it's possible to trigger them using a RPM post-install hook, I saw you already added code to do needed migrations after starting the app. So I better add the migration there.

@Armadill0

Copy link
Copy Markdown
Owner

Yes, this sounds like the best place to integrate it.

Thanks for the latest commit. The implementation is exactly what I had in mind. 😃 I hope I can merge the pull-request soon. My free time is currently strongly limited.

@schmittlauch

Copy link
Copy Markdown
Author

I'm short on time, too. But I want to complete this PR hopefully this month, or at least this year.

- split up rpoject into two subprojects with their own .pro files, but
sharing one spec/ yaml file
- project structure inspired by
https://github.com/amarchen/helloworld-pro-sailfish
- include testing instructions into README
@schmittlauch

Copy link
Copy Markdown
Author

I heavily changed the project structure to be able to incorporate unit tests into the project. It has it's quirks, but it is the best solution I found yet.
The actual unit test and the db migration still have to be written though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants