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

Merged pull request... #14

Closed
wants to merge 49 commits into from
Closed

Merged pull request... #14

wants to merge 49 commits into from

Conversation

jedie
Copy link

@jedie jedie commented Sep 4, 2017

It is time-consuming to maintain all changes in separate pull requests.

So i merged all changes from #9 #10 #11 and #13

try:
import parler
except ImportError:
PARLER_INSTALLED=False
Copy link
Member

Choose a reason for hiding this comment

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

Just set parler = None and use if parler: to check if it's available.

try:
import aldryn_translation_tools
except ImportError as err:
TRANSLATION_TOOLS_INSTALLED=False
Copy link
Member

Choose a reason for hiding this comment

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

same here, use aldryn_translation_tools = None for the except part. Also, remove the as err.

Copy link
Author

Choose a reason for hiding this comment

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

done in 0379bcb

@jedie
Copy link
Author

jedie commented Sep 7, 2017

Anything else that needs to be changed to merged?

@suutari-ai
Copy link
Member

Sorry, but this PR is too big. My time for this project is quite limited and reviewing all these unrelated changes in a single PR needs bigger "timeslot" than I have available for a while for this.

The start_date/end_date thing started to look ready for merging before you combined it here, so maybe just revive it in the separate PR?

@jedie
Copy link
Author

jedie commented Sep 8, 2017

That's a dilemma. My time is also limited. That's why I merged the changes.

And there's also another new thing coming up: I need a role "request publishing" and "accept/reject publish request" with short message exchange.

All this is too time-consuming to implement completely separately. Maybe i should made my fork independent and publish a new package to PyPi?!?

@JsseL
Copy link

JsseL commented Sep 8, 2017

@jedie I think there is value in focusing development into one repo. So if you want, we can still try to help you out in ironing the features. Just that, the smaller and more concise the PR, the easier it is to review and iterate on.

@jedie
Copy link
Author

jedie commented Sep 8, 2017

I think there is value in focusing development into one repo.

I agree with you. But the reactions times are very long or there is no response. e.g.:

#11 -> a simple bugfix + editorconfig, 8 days old -> No reaction
#10 8 days old, has some questions -> No reaction

Or the hint at #5 ... no reaction...

I think the interest in this project is very low.

@JsseL
Copy link

JsseL commented Sep 8, 2017

@jedie You are right, this is not actively maintained project, rather a occasionally maintained fork from an abandoned project.

So indeed, it might make sense to have your own fork to keep things going forward in your projects.

@codecov
Copy link

codecov bot commented Nov 14, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@3b3868f). Click here to learn what that means.
The diff coverage is 93.18%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #14   +/-   ##
=========================================
  Coverage          ?   50.99%           
=========================================
  Files             ?       11           
  Lines             ?      553           
  Branches          ?       74           
=========================================
  Hits              ?      282           
  Misses            ?      262           
  Partials          ?        9
Impacted Files Coverage Δ
publisher_tests/myapp/admin.py 28.22% <ø> (ø)
publisher/views.py 0% <0%> (ø)
publisher/version.py 100% <100%> (ø)
publisher_tests/myapp/models.py 75.62% <100%> (ø)
publisher/managers.py 86.11% <91.66%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b3868f...0cda7c9. Read the comment docs.

@jedie
Copy link
Author

jedie commented Nov 15, 2017

I have made a fork, merge all my changes and release this on PyPi as django-ya-model-publisher:

github: https://github.com/wearehoods/django-ya-model-publisher/
PyPi: https://pypi.org/project/django-ya-model-publisher/

@jedie jedie closed this Nov 15, 2017
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