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
Revert "Fixed Issue: #4940 Journal entries name can be renamed to blank" #752
Conversation
This reverts commit e05452a as the current behaviour is inconsistent across Sugar. An journal entry title does not need to be non-blank; the learner should be allowed to use a blank or empty title. The title can always be changed again. The default title is never blank. The title does not need to be unique; it is not a file name. Reference: https://bugs.sugarlabs.org/ticket/4940
|
The issue #4940 was well taken. The fix should have been applied.
Back in the day, a customer wanted to show off his hospital database.
His query was for patients older that 100. He was amazed to see huge
list. Turned out that date of birth had been left blank and so the epoch
date zero was used, January 1, 1601. Not one person suggested that since
that was the way SHAS worked, that was the way it should be. The IT
folks put a verification check in within hours. It took a little longer
to collect the correct dates of birth.
The title does need to be non-blank so the user looking at the Journal
View can tell which object is which. Why should a blank or empty title
not be considered a mistake?
Tony
…On 04/26/2017 12:13 PM, James Cameron wrote:
This reverts commit e05452a
<e05452a>
as the current behaviour is inconsistent across Sugar.
An journal entry title does not need to be non-blank; the learner
should be allowed to use a blank or empty title. The title can always
be changed again.
The default title is never blank. The title does not need to be
unique; it is not a file name.
Reference:
https://bugs.sugarlabs.org/ticket/4940
------------------------------------------------------------------------
You can view, comment on, or merge this pull request online at:
#752
Commit Summary
* Revert "Fixed Issue: #4940 Journal entries name can be renamed to
blank"
File Changes
* *M* src/jarabe/journal/listview.py
<https://github.com/sugarlabs/sugar/pull/752/files#diff-0> (4)
Patch Links:
* https://github.com/sugarlabs/sugar/pull/752.patch
* https://github.com/sugarlabs/sugar/pull/752.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#752>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAULkkLhDqeXyCBTigFR_UOmLzHN-EYJks5rzsRSgaJpZM4NIV2L>.
|
|
How do you propose a blank or empty title be set when the learner desires to set it? Why take agency from the learner? This isn't a health system. |
|
The user also has the agency to:
cd /
rm -rf *
although there is inconsistency in that this works in some deployments
not others.
In the current scenario, the user gives a non-blank title during save
but is not prevented from going to the Journal View and erasing the
title. About the same degree of difficulty as saving an object with a
blank title and going to the Journal View to set it. However, my guess
is that in most cases a blank title is a mistake not a wish.
Tony
…On 04/26/2017 02:19 PM, James Cameron wrote:
How do you propose a blank or empty title be set when the learner
desires to set it? Why take agency from the learner? This isn't a
health system.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#752 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAULktv9XOymULsMXjyg6PZYUi_KvcMzks5rzuIHgaJpZM4NIV2L>.
|
|
We're not here to force the learner into our mode of thinking; it might be a mistake, and if so let them fix it, or it might be intentional. |
|
Some Journal source code citations for empty title support in Sugar;
An interesting previous ticket; |
|
That is apparently exactly what you are doing, forcing the learner to
think like you do.
What about the learner who would like an alert when entering a blank
title? Does that thinking count for anything?
My mode of thinking is that #327 is what it is and should be merged
without any more ticky-tack. You have the setting so it will never be
inflicted on you.
That should be enough.
Tony
…On 04/26/2017 02:42 PM, James Cameron wrote:
We're not here to force the learner into our mode of thinking; it
might be a mistake, and if so let them fix it, or it might be intentional.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#752 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAULklXHX_tUBa7XzGJFeM1ItQB8zRphks5rzudbgaJpZM4NIV2L>.
|
|
Well, we're hearing a lot from you, but I'm hoping other people will say what they think. I'll hold off on #327 some more now that I can see there is not general agreement on the purpose of the feature; you saying it is to avoid blank or empty titles, and me saying it is to force reflection and choice of a title. |
|
The purpose of the feature has been described over and over again. It is
not about files with blank titles. Utkarsh chose to alert the user who
clicked on ok with a blank title. Perfectly reasonable programming decision.
The technical issues you raised were fixed by Utkarsh quickly and
apparently to your satisfaction. Your refusal to merge the PR is now
based sole on your personal prejudices. There is a setting so that the
feature by default is not enabled, so it is really difficult to
understand your concerns.
You do not agree with it. Walter said go ahead. Sebastian says go ahead.
Utkarsh says go ahead. I say go ahead.
Tony
…On 04/26/2017 02:58 PM, James Cameron wrote:
Well, we're hearing a lot from you, but I'm hoping other people will
say what they think. I'll hold off on #327
<#327> some more now that I can
see there is not general agreement on the purpose of the feature; you
saying it is to avoid blank or empty titles, and me saying it is to
force reflection and choice of a title.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#752 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAULkg9ZAaqchH9FlmczI423FmZZUu3Dks5rzusagaJpZM4NIV2L>.
|
|
I don't think they have appreciated the finer points that are discovered on careful review of the patch, and it is those finer points I've asked Utkarsh to respond to. No need to list them again, as they are over on sugarlabs/sugar-toolkit-gtk3#327 |
|
What 'finer points' do you feel are outstanding?
Here is the checklist -
* Flake8 in alerts.py fixed
Please note - There are already existing flake8 errors before this
PR. I would suggest fixing them in a separate PR rather than
including in this one.
* Branch rebased to the latest commit.
* Variable name use for gsettings is |'_save_as_enabled'| since
another variable with |_save_as| name has already been used in lot
of places. And there are other existing gsettings variables in Sugar
which go by this naming style - |gsetting_name_enabled| so should
not be a problem.
* Commit message updated with reference page
* Check settings everytime rather than save it at startup
* Add new |GConf| value to sugar schema for Save-As
PR - sugarlabs/sugar#751 <#751>
* Add save-as functionality to |sugar-toolkit|
PR - sugarlabs/sugar-toolkit#4
<sugarlabs/sugar-toolkit#4>
Utkarsh shows that he has addressed all of the issues.
Tony
…On 04/26/2017 03:52 PM, James Cameron wrote:
I don't think they have appreciated the finer points that are
discovered on careful review of the patch, and it is those finer
points I've asked Utkarsh to respond to. No need to list them again,
as they are over on sugarlabs/sugar-toolkit-gtk3#327
<sugarlabs/sugar-toolkit-gtk3#327>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#752 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAULkp1zFxICgK9uNbGe8PNJKpzggkgHks5rzve0gaJpZM4NIV2L>.
|
This reverts commit e05452a as the current behaviour is inconsistent across Sugar.
An journal entry title does not need to be non-blank; the learner should be allowed to use a blank or empty title. The title can always be changed again.
The default title is never blank. The title does not need to be unique; it is not a file name.
Reference:
https://bugs.sugarlabs.org/ticket/4940