Getekid
commented
4 years ago Hello, thanks for resubmitting the PR, please keep the language to English since that's the repo's language (my French are also a bit poor) and I'll try to keep it simple. Same rule goes for code comments.
Welcome then, in general just so you know, you'll notice that this isn't a very professional project either, it has some best practices but misses others (e.g. tests).
First let me start with a best practice:
- One PR should have one purpose, if you have multiple fixes please separate them in different PRs, this way I can approve your bug fixes (which are valid) and discuss the new features separately.
Now for the migration. Please have a look at the migrations documentation. In short
- Do NOT alter
release_0_5_0.phpas it will break existing installations - Create a new file called
release_1_1_0.phpand add your changes there- If you can, add
release_0_5_0as a dependency
- If you can, add
Another thing I hadn't seen. Disabling db login breaks the site when "DB Login" is on. If it breaks the admin login then maybe changing the username/password check to both instead of one of them (i.e. change the OR check to AND).
And a last thing, it's a bit bad timing but I updated the repo to work with composer. v2.0.0 has all files to the root instead of in the getekid/cas directory.
So next steps:
- If you can fix the migration and db login issues, I'll approve the PR, otherwise please submit a PR for each separate feature/bug fix so that I can better manage them
- If you can rebase/resubmit the PR for v2.0.0 it would be awesome
Thanks again for your time and fixes, much appreciated
pingou2712
Ok, i submit an another PR =) with: -> add option for SingleSignOut
->minor changement of gestion of bug : replace of : if (!preg_match('/[.\d-_abcdefghijklmnopqrstuvwxyz\/]*/', $this->config['cas_uri'])) to : if (!preg_match('/[.\d-_abcdefghijklmnopqrstuvwxyz\/]+/', $this->config['cas_uri'])) prevent null cas_uri for test =)
In concern : "Your DB update should exist in a file of its own (e.g. migrations/release_1_1_0.php. Some people (not many, but still) are using this plugin and this way phpBB knows to apply only the new DB changes." . Heeeeuuuuu.... I'm not sure to understand what you expect... I'm french (with very poor english) and new with all of that... but I just delete session of user of existing table... no need migration, i think?