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

Adds ability to override the django runner class #130

Merged
merged 15 commits into from
Jan 10, 2022

Conversation

kingbuzzman
Copy link
Contributor

@kingbuzzman kingbuzzman commented Jan 8, 2022

Addresses issue: #122
Replaces: #125

@kingbuzzman
Copy link
Contributor Author

@bittner Still have 2 outstanding issues:

  • the CI in behave-django fails to run.
  • bahave hasn't published 1.2.7

@bittner
Copy link
Member

bittner commented Jan 8, 2022

Don't worry about Travis CI. I'll replace that by GitHub Actions tonight.

As for the Behave release, we have a much bigger problem: Only @jenisys can issue releases to PyPI, and it's unclear why he's waiting so long for it. (In the past this was all about Windows compatibility, but now ... not sure.)

Maybe you could (try to detect the Behave version and) abort execution when the feature is not supported, because the installed Behave package is not new enough?

@bittner
Copy link
Member

bittner commented Jan 9, 2022

The migration from Travis CI to GitHub Actions is now complete. I also added isort as a pipeline job to enforce more code style. The changes are merged into the main branch (yes, main not master; I also switched the default branch).

You need to fix a minor conflict in the behave management command module now due to changes on the imports, I suppose. Sorry for the inconvenience! – Looking forward to a rebased PR that I could merge. 👀 💯

@kingbuzzman
Copy link
Contributor Author

@bittner looks like my workflow is stuck in limbo awaiting your approval ;)

setup.py Show resolved Hide resolved
@kingbuzzman
Copy link
Contributor Author

@bittner I "fixed" the requirement issue, I still don't like that 1.2.7 hasn't been released, but at least tox doesn't have an issue with it anymore. Don't know what you want/can to do about it... The remaining issue now is the CI not running.

Copy link
Member

@bittner bittner left a comment

Choose a reason for hiding this comment

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

LGTM, merging.

HISTORY.rst Outdated Show resolved Hide resolved
Co-authored-by: Peter Bittner <django@bittner.it>
@kingbuzzman
Copy link
Contributor Author

kingbuzzman commented Jan 10, 2022

Well I would say it's "ready", my only concern is the requirements.txt, I suppose it's easy enough to create another PR to address it later on... balls on your court, chief ;) .

@bittner
Copy link
Member

bittner commented Jan 10, 2022

Ooops! What seems to be missing, actually, is updating the documentation. Can you add the option to the configuration chapter? With a few words of which problem it solves and how it is meant to be used.

In the end, would you mind squashing all the changes, so that I can merge one (or two) pristine commit(s) bearing your name? Or should I simply let GitHub do the squashing?

@kingbuzzman
Copy link
Contributor Author

What seems to be missing, actually, is updating the documentation. Can you add the option to the configuration chapter? With a few words of which problem it solves and how it is meant to be used.

on it

In the end, would you mind squashing all the changes, so that I can merge one (or two) pristine commit(s) bearing your name? Or should I simply let GitHub do the squashing?

I like the squashing option better.

@bittner
Copy link
Member

bittner commented Jan 10, 2022

Well I would say it's "ready", my only concern is the requirements.txt, I suppose it's easy enough to create another PR to address it later on... balls on your court, chief ;) .

I'll take the opportunity to go down the stony way of trying to convince @jenisys to push a new release to PyPI. This is dangerous as it could keep this project trapped in the state that Behave is in for years now. But, in the end, behave-django is just a development tool, isn't it? We can easily leave the dependency on the development version of Behave. As long as we want.

What do you think? Any other thoughts?

@kingbuzzman
Copy link
Contributor Author

Well I would say it's "ready", my only concern is the requirements.txt, I suppose it's easy enough to create another PR to address it later on... balls on your court, chief ;) .

I'll take the opportunity to go down the stony way of trying to convince @jenisys to push a new release to PyPI. This is dangerous as it could keep this project trapped in the state that Behave is in for years now. But, in the end, behave-django is just a development tool, isn't it? We can easily leave the dependency on the development version of Behave. As long as we want.

What do you think? Any other thoughts?

I agree with everything you just said 👍

@kingbuzzman
Copy link
Contributor Author

@bittner ..why do you have to accept every CI commit now? Is there a way you can set it to run always? Are you charged per CI task?

@bittner
Copy link
Member

bittner commented Jan 10, 2022

@bittner ..why do you have to accept every CI commit now? Is there a way you can set it to run always? Are you charged per CI task?

I think this is just as long as I haven't merged "your first contribution". This sounds silly, because you have contributed a lot, I feel, but GitHub has no evidence (no commit from you in the code base) yet. Don't worry, we're about to fix that, too.

HISTORY.rst Outdated Show resolved Hide resolved
docs/configuration.rst Outdated Show resolved Hide resolved
docs/configuration.rst Outdated Show resolved Hide resolved
docs/configuration.rst Outdated Show resolved Hide resolved
@bittner bittner merged commit a37b844 into behave:main Jan 10, 2022
@bittner bittner changed the title Adds ability to modify the django runner class Adds ability to override the django runner class Jan 10, 2022
@kingbuzzman kingbuzzman deleted the feature/django-runner-class branch January 10, 2022 15:15
@kingbuzzman
Copy link
Contributor Author

Very nice. Thank you @bittner

@bittner
Copy link
Member

bittner commented Jan 10, 2022

Thank you so much, @kingbuzzman! 🥇 💯 🏆

As a bonus you get free, automatic CI runs for all your changes with your next PRs!! 😏

@bittner
Copy link
Member

bittner commented Jan 10, 2022

BTW, have you seen #129? Maybe you want to "apply"? 🤓 😃

I would appreciate that. Dead seriously.

@kingbuzzman
Copy link
Contributor Author

I did see it, I wouldn't mind as a soft/light co-maintainer

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