Refactor ensure DreddHooks::Server can be used as library #10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There is a quite widespread convention that the default Rake task runs the test suite (Travis CI uses it for example). Running the Cucumber features using running 'cucumber' (resp. 'bundle exec cucumber') is still possible. Also more Rake tasks could be added for convenience, and the debug scenarios cold be run using 'rake cucumber:debug' for example.
As a Ruby developer new to this project In order to get started quickly And know which part of the test suite is intended to be run daily When I run rake Then I want to see the entire test suite output
As a developer who considers updating Dredd Hooks Ruby In order to have a clear idea of which changes I should expect I want them described in a CHANGELOG
This file SHOULD NOT be ignored, because it is the only way to ensure the bundle consitency accross different repositories. It should never be edited by hand either, and any change to the Gemfile or gemspec should be committed with the corresponding Gemfile.lock - run 'bundle install' to update the lock.
The 'bundle exec rake' command did fail with message:
$ bundle exec rake
Feature: Execution order
Killed: 9
I suspect that the error was shadowed because the Gemfile.lock was missing,
but I am not sure exactly why. (Travis seems to be using the new
'features/support/env.rb and still doing fine.)
Anyway, the error is present in my development env. since 8dc3e26
and appeared in Travis CI when I added the Gemfile.lock.
See https://travis-ci.org/gonzalo-bulnes/dredd-hooks-ruby/jobs/125324433
This commit partially reverts 8dc3e26
The TCP Server feature starts a server but doesn't stop it. This breaks the next test suite run unless the server is manually killed, which is unpractical. I think it's best to remove the entire feature until findinf a proper fix to this issue and 7330c74
Bump version number
Minor remove excessive installation details
Installing gems manually ('gem install dredd_hooks') is always possible,
however, if you who needs it to be mentionned in the README, you are
probably not familar with Ruby and should use a Gemfile.
As a developer In order to be able to update my dependencies automatically And to be sure their maintainers care about their API I want their version numbers to convey meaning
Update copyright notice See https://opensource.org/licenses/MIT
The builds on my account are up-to-date, and the README will anyway be edited when merging upstream.
As a dredd-hooks-ruby user In order to know the project status And be able to read the documentation in a terminal I want its README to be formatted for source readability And to display build, dependencies, code climate and inline doc badges
Since the contribution is substantial, and following the usage instructions described in https://www.gnu.org/licenses/gpl-howto.html
These lists of files are not modified often, and the Git versions are cryptic enough to allow useless files to be added to the gem.
The entry points for the gem are: - the executable, which depends on the FileLoader and Server - the include in the hooks files, which I suggest taking care of later I think that the executable should probably not know about the internal organization of the gem, but it's mostly matter of appreciation (YMMV).
The FileLoader.unique_paths is not part of its API and should not be used by other classes/modules.
Asserting on the objects class is not used much in Ruby, and in this case it was used in case 'hooks' were nil. Instead, 'hooks' could default to an empty array (which conveys more meaning that nil) and it would always responds to "#each". The solution only builds on top of the existing duplication, but that's a different problem.
Minor use parens around arguments in method definitions and non-trivial method calls.
Minor refactor use Array#inject to group statements Minor use parens in multi-arguments method calls
Minor use parens in method definition
The server is already interacting with the client in the run loop, and I see no reason iwhy the message processing method should know about it.
I'm not that sure about this one, but I believe the MESSAGE_DELIMITER is a "full duplex" convention between the server and the client and that it is not a coincidence that the same message delimiter is used in the client requests and in the server responses.
As a developer In order to keep simple the code which is likely to be extended And keep apart the different responsibilities it assumes I want the 'DreddHooks::Serveri' to be modular
When adding support for new hooks, both the hooks definition and the events definition needs to be updated. It makes sense to me to keep them in the same file. As the events definition refers to the hooks names, in order to make maintenance easier, I think it also makes sense to check consistency, and break the test suite when required definitions are missing.
As a developer In order to make maintenance easier And ensure adding support for new Dredd events is as simple as possible I want the DreddHooks::Server::EventsHandler to be DRY
Remove Bundler dependency I might be wrong, but I think declaring bundler as a dependency is superfluous (e.g. its version is not locked in the Gemfile).
This reverts commit ae792ba due to failures in the Ruby 1.9.3 build on Travis CI. I'm not sure why the Ruby 1.9.3 build fails (while the Ruy 2.3.0 succeeds) but truth is this update can be omitted for now. See https://travis-ci.org/gonzalo-bulnes/dredd-hooks-ruby/builds/127012201
This refactoring should allow to start using Thor to build the CLI with minimal disruption. Using Thor would change the API (starting the server would require to dredd-hooks-ruby start instead of dredd-hooks-ruby) but would allow to build a properly documented CLI (dredd-hooks-ruby help and so on...) with minimal effort. See http://whatisthor.com
Minor refactor fix useless variable assignation
...provide default argument to 'Hash#fetch' instead.
Minor move informative output to STDOUT. Injecting the error and output steams allows to configure them and to write specs for their messages (eventually).
As a developer In order to be able to start the Ruby hooks server programatically I want to keep all the Ruby hooks server business logic in a single directory In order to to be able to improve its CLI independently I want its CLI to be a first-class citizen among the code
Ensuring that the output doesn't get buffered is particularly relevant once the server is started because Dredd relies on it. In order to be able to start programatically a server in the best conditions, that step should be part of the server starting procedure. Using a buffered output while loading the hookfiles, on the other hand poses no particular problem.
Unless I'm wrong, declaring bundler as a dependency is superfluous (e.g. its version is not locked in the Gemfile).
|
I will re-open this PR against the canonical repository when it is ready. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a developer
In order to provide multiple uses to my gem
I want its code to be usable without its command-line interface
See apiaryio#5