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

Configmgr improvements #186

Merged
merged 22 commits into from Aug 3, 2013
Merged

Configmgr improvements #186

merged 22 commits into from Aug 3, 2013

Conversation

Chris--S
Copy link
Collaborator

all so far is pretty safe/minimal.

more is possible/likely

@splitbrain
Copy link
Collaborator

I'd be fine with merging this right away, but since it's not really bug fixing it should probably wait til after the release?

@Chris--S
Copy link
Collaborator Author

The second last commit is a bug, but one that would be very easy to replicate. The rest don't make a huge difference either way. As I say above, I'm (sort of) considering planning doing more, I put this up to hopefully get some discussion on ideas at a developer irc chat.

Chris--S and others added 14 commits February 24, 2013 12:39
IE9 send different HTTP_ACCEPT_LANGUAGE header on ajax request. This causes different results from auth_browseruid. This patch removes the HTTP_ACCEPT_LANGUAGE from the browser id calculation.
changed nonexistant actionOk to actionOK
In the case of a failed authentication initialization, the
authentication setup was simply continued with an unset $auth object.
This restores the previous behavior (before merging #141) of simply
returning after unsetting $auth. Furthermore this re-introduces the
check if $auth is set before checking $auth and removes a useless
check if $auth is true (could never be false).
This reverts parts of the changes from #154: Before merging the pull
request, a depth of 1 returned just the pages in the root namespace.
With the changes in the pull request, a depth of 1 also returned pages
in subnamespaces of the root namespace (as it was also tested in the test case).
This reverts this part of the changes and a depth of 1 returns just the
pages in the root namespace again.
This adds $INPUT in all places where it was still missing and available.
$INPUT is now also used in places where using $_REQUEST/... was okay in
order to make the code consistent.
@Chris--S
Copy link
Collaborator Author

hmm.

I did git cherry-pick to apply the xmlrpc caution fix to master, then rebased my local copy of this branch. Why does it end up like this (it wouldn't let me push without a merge) - listing all the patches from master and not simply removing the commit which now exists in master?

@splitbrain
Copy link
Collaborator

From my experience:

  • never ever do a rebase on anything that has been pushed somewhere, it always ends up weird
  • a cherry picked patch has a different ID than the orginal patch and thus is treated as a different patch by git
  • cherry picked patches are usually detected just fine on a merge, so there's no need to "clean up" the feature branch for later merging

@Chris--S
Copy link
Collaborator Author

In effect, you're saying, once I'd pushed the cherry-pick, I shouldn't have done(/didn't need to do) the rebase and merge?

I'll redo the branch/PR later to get it looking cleaner.

@splitbrain
Copy link
Collaborator

conflicts need to be resolved, but should be merged

@splitbrain
Copy link
Collaborator

@Chris--S can you update this so it's mergable?

Conflicts:
	inc/auth.php
	inc/template.php
	lib/plugins/authad/lang/zh/settings.php
	lib/plugins/authldap/lang/en/settings.php
	lib/plugins/authldap/lang/zh/settings.php
	lib/plugins/authmysql/lang/zh/settings.php
	lib/plugins/config/settings/config.class.php
	lib/plugins/usermanager/admin.php
@Chris--S
Copy link
Collaborator Author

Chris--S commented Aug 2, 2013

merge conflict removed. should be good to go...

splitbrain added a commit that referenced this pull request Aug 3, 2013
@splitbrain splitbrain merged commit 4918284 into master Aug 3, 2013
splitbrain pushed a commit that referenced this pull request Apr 9, 2020
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.

None yet

4 participants