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

More cleanup in testing #88

Merged
merged 5 commits into from
Dec 1, 2017
Merged

More cleanup in testing #88

merged 5 commits into from
Dec 1, 2017

Conversation

bast
Copy link
Member

@bast bast commented Dec 1, 2017

Another quarter million lines of text gone.

@bast bast requested a review from heikef December 1, 2017 11:51
@heikef
Copy link
Contributor

heikef commented Dec 1, 2017

I assume the "nuclear waste" is only removed regarding the test example. The out commented keywords are important and should be kept as an option for the input. For the specific test case we look at here they are not needed indeed. - Yes, this is the case. -))

@bast
Copy link
Member Author

bast commented Dec 1, 2017

No, outcommented code does not belong here. Keywords may and will change and then the commented code is confusing both for users (why does this not work anymore?) and developers (will this be needed in future? but nobody is using it).

For me outcommented input examples are a symptom for the following frequent problems:

  • input format is not intuitive
  • keywords are not intuitive
  • documentation is not helpful

@heikef
Copy link
Contributor

heikef commented Dec 1, 2017

Thanks for the cleanup.

@heikef heikef merged commit 364dd4f into qmcurrents:master Dec 1, 2017
@heikef
Copy link
Contributor

heikef commented Dec 1, 2017

I think therefore we have issue #84.

From a user's point of view. The keywords are useful and needed. The input format is very intuitive when you understand the "gimic logic". Documentation can always be optimized. At the moment I would not call it unhelpful.

I want to keep the keywords!

@bast
Copy link
Member Author

bast commented Dec 1, 2017

You want to keep out-commented keywords in the tests? Why not put them in some examples? The tests should test one thing only. I find it very confusing to have them in the tests.

@heikef
Copy link
Contributor

heikef commented Dec 1, 2017

No, I do not want to keep out-commented stuff in the tests. I was pointing out that the keywords that are out-commented are still relevant and should not be in general removed from the input. This is not related to this specific test. Apparently that was not clearly stated. Sorry for this. Hope this is clear now.

@bast
Copy link
Member Author

bast commented Dec 1, 2017

OK I was of course not planning at removing any keywords :-) Only removed out-commented lines.

@heikef
Copy link
Contributor

heikef commented Dec 1, 2017

Good. -)) Just wanted to be sure about this. One never knows.

@bast bast deleted the radovan/refactor-tests branch December 1, 2017 14:34
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