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

Fixed issue #254 - auto "fireuping" sup-config, if there are no any configured sources #358

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

tradzik
Copy link

@tradzik tradzik commented Dec 28, 2014

Added automatic fire-uping "sup-config" if there are no any configured sources.

@eMBee
Copy link
Contributor

eMBee commented Dec 29, 2014

test how it works with:

  - no .sup dir
  - a previous config.yml but no sources.yml
  - both existing, but perhaps sources empty.

@tradzik
Copy link
Author

tradzik commented Dec 29, 2014

Ok, @eMBee . I will make these tests. What should I do after that? Is there anything wrong?

@eMBee
Copy link
Contributor

eMBee commented Dec 29, 2014

report on how each test went and, see if the script was started in every case.
also check maybe if config.yaml was changed after running the script.

if the tests pass, then there is nothing to do, otherwise, look into what needs to be fixed to make that test work.

@tradzik
Copy link
Author

tradzik commented Dec 29, 2014

Thank you for your answer. I will finish tests ASAP and give results there

@tradzik
Copy link
Author

tradzik commented Dec 29, 2014

First test report: script started without any problem

@tradzik
Copy link
Author

tradzik commented Dec 29, 2014

Second test: mistakes found. Runtime error. Improving

@tradzik
Copy link
Author

tradzik commented Dec 30, 2014

Now, after my changes, all tests work

@gauteh
Copy link
Member

gauteh commented Dec 30, 2014

make sure recovering from ncurses setup work. starting and stopping
external programs from sup does not work without a bit of care.

On Tue, Dec 30, 2014 at 11:16 AM, Tymon Radzik [email protected]
wrote:

Now, after my changes, all tests work


Reply to this email directly or view it on GitHub
#358 (comment).

@@ -152,10 +152,18 @@ end
module_function :start_cursing, :stop_cursing

Index.init
Index.lock_interactively or exit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you doing this? Make sure you understand all the implications of the changes you are doing as well as testing it. Changing this line is not something you do without careful thought and a good overview of how sup works.

@tradzik
Copy link
Author

tradzik commented Dec 31, 2014

I have fixed that as in disscussion. Now everything should be ok. Please also take a look on #359

@tradzik tradzik changed the title Fixed issue #254 Fixed issue #254 - auto "fireuping" sup-config, if there are no any configured sources Jan 13, 2015
@eMBee
Copy link
Contributor

eMBee commented Jan 13, 2015

this works, but i wonder if we need do consider more cases.

right now, if i go through config, but explicitly do not add sources, it will ask me again every time until i have at least one source.
is it conceivable to use sup without sources? for sending only maybe? or do i need to add sent as a source? shouldn't that be in the sources by default?

i think that once config has been called, it should not be called again unless explicity invoked.
this could be solved like so: config is called, it sets a flag in the config about being called. sup starts, it checks the flag. if there, it moves on. if missing, sup calls config.

@eMBee
Copy link
Contributor

eMBee commented Jan 18, 2015

as mentioned before: in commit messages: don't write "Added eMBee's suggestion" but explain what the actual change/suggestion is

@eMBee
Copy link
Contributor

eMBee commented Jan 18, 2015

to sup devs: is a file to mark configured state ok? or is there a better way to mark that? some variable that can be saved?

@gauteh
Copy link
Member

gauteh commented Jan 18, 2015

What about just checking if it matches default config and whether there are any sources, then ask the user whether he wants to launch sup-config (I can't think of any use case where the user would want to work without any sources, any previously indexed messages would be useless since they rely on the source to be able to open them). If there is, then there could be a config option that says something like 'do-not-ask-to-start-sup-config-again'.

@@ -156,6 +156,16 @@ Index.lock_interactively or exit

begin
Redwood::start
Redwood::SourceManager.load_sources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not done twice now (in case of configured)?

@gauteh gauteh assigned eMBee and unassigned eMBee Feb 11, 2015
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.

4 participants