Improvements around changing owners #792
Conversation
hasegeli
force-pushed
the
master
branch
6 times, most recently
from
973c447
to
d1f207f
Compare
|
Any luck looking at this? It would be better to do so, before releasing the |
| $tablespace = undef, | ||
| $template = 'template0', | ||
| $encoding = $postgresql::server::encoding, | ||
| $locale = $postgresql::server::locale, | ||
| $istemplate = false, | ||
| $connect_settings = $postgresql::server::default_connect_settings, | ||
| $change_ownership = false, |
There was a problem hiding this comment.
Removing this parameter is a breaking change that might cause existing configurations to break. It is better for users to accept the parameter, but not act on it, except for emitting a warning that it will go away. Newer versions of stdlib have a deprecation method to do so in a friendly way.
There was a problem hiding this comment.
I don't think this argument has been released yet. Do you think it is still necessary to accept it?
There was a problem hiding this comment.
For reference, this is a revert of 82c6716 which indeed was never released.
hasegeli
force-pushed
the
master
branch
4 times, most recently
from
b9897bb
to
33e8133
Compare
|
I made the commits really clean by first reverting the existing changing owner implementation. |
This reverts commit 82c6716. It will be added back with a cleaner design with the following commits.
hasegeli
force-pushed
the
master
branch
2 times, most recently
from
3b5dbbb
to
c0ff556
Compare
This can be considered a bug fix, because the old names would conflict when there are schemas with the same name on different databases.
This also automatically requires the owner role on the database class.
|
This looks SO MUCH better! 🎉 You do seem to have missed one place in the tests where the expected title has changed. I fully expect @ntpttr be satisfied with the changes too, but he should be aware of them. |
|
I fixed it. Thank you for the review. |
|
Thank you for looking at it. |
Improvements around changing owners
No description provided.