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

Nothing is new_ #1753

Closed
4 tasks done
wardi opened this issue Jun 9, 2014 · 10 comments
Closed
4 tasks done

Nothing is new_ #1753

wardi opened this issue Jun 9, 2014 · 10 comments
Assignees

Comments

@wardi
Copy link
Contributor

wardi commented Jun 9, 2014

Targets for renaming:

@wardi wardi self-assigned this Jun 9, 2014
@tobes
Copy link
Contributor

tobes commented Jun 10, 2014

:)

All I would say based on experience is that git does not like file renames when they are ambiguous. eg. if we change test -> tests_legacy and then test_new -> tests then this will create lots of noise between branches.

new_authz.py can probably become authz.py as that file has not existed for some time although this may risk breaking badly written extensions. maybe it should just move to lib.authz.py instead.

Templates:
As you know these will break extensions. If I had my way I would just go straight to your final version 3 in #1659 the templates are too complicated by far so why not just rationalize them.

Also delete every legacy template and every failing test caused by this (yes you would loose test coverage but test coverage of unused templates/workflows is sort of pointless) - @seanh will never agree to this I suspect.

@seanh
Copy link
Contributor

seanh commented Jun 10, 2014

I'd be happy to delete the legacy templates and their tests, actually, as I doubt the tests are very helpful. I'd be even happier to see new frontend tests though!

I agree with all the renaming if it can be done practically

@tobes
Copy link
Contributor

tobes commented Jun 10, 2014

Do we also bite the bullet and change templates/package/package_form.html to templates/dataset/dataset_form.html that would be the real goal :)

@joetsoi
Copy link
Contributor

joetsoi commented Jun 10, 2014

+1, just a warning that we might break extensions that have imported new_tests.

e: https://github.com/ckan/ckan/blob/master/ckan/ckan_nose_plugin.py#L20

when renaming new tests, this needs to be fixed as well.

@wardi
Copy link
Contributor Author

wardi commented Jun 13, 2014

@tobes I don't think that's likely to happen until the next complete template rewrite (same for action api package methods) but I do support it

@wardi
Copy link
Contributor Author

wardi commented Jun 16, 2014

@tobes lib.authz is a good suggestion, it seems untidy to have it at the root and that would match better with lib.search etc.

But it's always seemed funny to me that things that call other parts of ckan are under 'lib' though. 'lib' to me says "separate library we've pulled in to our source tree". The authorization is tied deeply with the rest of ckan.

@amercader
Copy link
Member

Do we want to bite the bullet on some of these? I personally would like to see the new_authz and new_tests renamed before 2.4, but if it causes too much bother we can defer.

As per deleting the old templates, see #1772

@wardi
Copy link
Contributor Author

wardi commented Oct 23, 2014

Yes, I would like to try to knock some of these off before they are even more entrenched

@wardi
Copy link
Contributor Author

wardi commented Apr 1, 2015

Thank you @vitorbaptista @amercader @davidread @joetsoi !!!

@wardi wardi closed this as completed Apr 1, 2015
@davidread
Copy link
Contributor

👍

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

No branches or pull requests

6 participants