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

get tests running / convert to bundler #101

Closed
wants to merge 3 commits into from

Conversation

grosser
Copy link
Contributor

@grosser grosser commented May 2, 2019

before: fun errors like this

rake
rake aborted!
LoadError: cannot load such file -- hoe

@tenderlove @drbrain

closes #97
closes #93
closes #86

@grosser
Copy link
Contributor Author

grosser commented May 4, 2019

@tenderlove @drbrain 🛎

Copy link
Collaborator

@tenderlove tenderlove left a comment

Choose a reason for hiding this comment

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

I don't think we want the lock file or bundle configuration to be part of manifest.txt do we? It means they'll get shipped with the gem.

@grosser
Copy link
Contributor Author

grosser commented May 5, 2019

the tests blew up when I did not add them 🤷‍♂
... can I just kick out all the manifest/hoe stuff so it uses bundlers rake tasks for release ?

+vendor/bundle/specifications/rake-12.3.2.gemspec
rm Manifest.tmp
rake aborted!
Command failed with status (1): [diff -du Manifest.txt Manifest.tmp...]
vendor/bundle/gems/hoe-3.17.2/lib/hoe/debug.rb:92:in `block in check_manifest'
vendor/bundle/gems/hoe-3.17.2/lib/hoe.rb:921:in `with_config'

@grosser
Copy link
Contributor Author

grosser commented May 5, 2019

... and atm it ships .autotest / Rakefile / test* which also makes no sense :D

@grosser
Copy link
Contributor Author

grosser commented May 5, 2019

... added coversion to gemspec so tests pass and it's more modern

@grosser grosser changed the title get tests running get tests running / convert to bundler May 5, 2019
@grosser
Copy link
Contributor Author

grosser commented May 6, 2019

@tenderlove @drbrain 🛎

license 'MIT'

rdoc_locations <<
'docs.seattlerb.org:/data/www/docs.seattlerb.org/net-http-persistent/'
Copy link
Owner

Choose a reason for hiding this comment

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

The docs are no longer published and I don't think bundler's publishing tasks have this capability for a rake release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can we use a more standard docs host ... or just remove hosting docs in favor of directing people to just read the docs/readme on github ? (removes lots of complications ... and this is a tiny gem anyway, so no need for fancy docs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... or leave that for "whenever someone care enough to fix it" ... because I'd love to actually fix the code and not be blocked by this docs issue :(

Copy link
Owner

Choose a reason for hiding this comment

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

so no need for fancy docs

I spent a lot of time writing comprehensive documentation that's accessible from standard tools. Pretending like they don't exist and that they'll maintain themselves means they'll decay.

Reading documentation out of source files isn't discoverable or easy for people who aren't experienced rubyists or aren't familiar with the library. Having a first-class experience as an introduction to a library is essential.

I've fixed the docs issue by fixing hoe-travis (1.4.1 release) and creating seattlerb/hoe#89. The tests now run and pass on master on travis.

README.rdoc Show resolved Hide resolved
@drbrain
Copy link
Owner

drbrain commented May 7, 2019

I think the first two README diff hunks and the test change are great.

I think adding a test run with net-http-pipeline is useful, but it can't be a dependency. I think it can be added to the Rakefile's spec block as:

  dependency 'net-http-pipeline', '~> 1.0' if ENV['TRAVIS_MATRIX'] == 'pipeline'

And:

matrix:
  include:
    - rvm: "2.6"
      env:
        TRAVIS_MATRIX: pipeline

If you want to try this out for me. (If you have a better name than "TRAVIS_MATRIX" for the environment variable that would be great.)

Switching to bundler's tasks is not acceptable. If we need to link to or write a "getting started with hoe" document I'm happy to write that.

@grosser
Copy link
Contributor Author

grosser commented May 7, 2019

  • lots of people successfully use bundler gem tasks, it surely has more users than hoe -> would make contributing easier
  • to use this gem from github we need a gemspec that is checked in
  • net-http-persistent is not a beginners ruby library ... very few people look up it's docs, and even if they do github readme would work well enough for most usecases ... and whoever wants to have deeper knowledge can go read the source
  • I'll revert pipeline since I'd rather not block this PR further
  • I'll keep this here for future reference and open a new PR

@grosser grosser closed this May 7, 2019
@drbrain
Copy link
Owner

drbrain commented May 7, 2019

lots of people successfully use bundler gem tasks, it surely has more users than hoe -> would make contributing easier

rake release in hoe does:

  • Removes any working files
  • Checks the version you want to release matches the version you have in source
  • Checks the files you want to release are committed
  • Ensures there are no garbage files
  • Tags the release
  • Pushes the tag
  • Builds the gem
  • Pushes the gem
  • Pushes documentation
  • Has a library of plugins for many other capabilities outside release

AFAICT the bundler tasks do only four of those (tag, push tag, build, push gem), and doesn't offer any hooks to perform any of the others.

to use this gem from github we need a gemspec that is checked in

I don't support unreleased software. My time is limited and chasing down a bug in unfinished work further distracts from that time.

If you want to use this from GitHub you can fork, generate one with rake debug_gem, and commit it.

This is also safer as it's possible for malicious parties to somehow compromise me or another contributor and commit malicious code to the repository.

net-http-persistent is not a beginners ruby library ... very few people look up it's docs, and even if they do github readme would work well enough for most usecases ... and whoever wants to have deeper knowledge can go read the source

Granted HTTP is complicated, I did limit the interface to Net::HTTP to make Ruby + HTTP easier to use safely. Saying "go read the source" is a step away from this goal and makes it more hostile to beginners. I put in some effort to document the various options and configuration settings to make it easier to know what to change and not change. Hiding this work makes support harder and disrespects my users' time.

I'll revert pipeline since I'd rather not block this PR further

I'll experiment with the matrix then

I'll keep this here for future reference and open a new PR

Great!

@grosser
Copy link
Contributor Author

grosser commented May 7, 2019

gemspec makes it possible to install forks from local disk and github, which simplifies debugging / development ... also integrates with Gemfile to install gems

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.

3 participants