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

CRM-20764 Allow clean URLs with CiviCRM on Backdrop #10674

Merged
merged 2 commits into from
Nov 15, 2017
Merged

CRM-20764 Allow clean URLs with CiviCRM on Backdrop #10674

merged 2 commits into from
Nov 15, 2017

Conversation

herbdool
Copy link
Contributor

@herbdool
Copy link
Contributor Author

@totten @colemanw here's hoping this is a fairly simple fix.

@seamuslee001
Copy link
Contributor

@herbdool Herb can you rebase this branch against upstream master by doing git rebase -i upstream/master and just picking off commit e4bd329

@@ -443,6 +443,9 @@ if (!defined('CIVICRM_CLEANURL')) {
if ( function_exists('variable_get') && variable_get('clean_url', '0') != '0') {
define('CIVICRM_CLEANURL', 1 );
}
elseif { function_exists('config_get') && config_get('system.core', 'clean_url') != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@herbdool there is also a php error here

PHP Parse error:  syntax error, unexpected '{', expecting '(' in /home/jenkins/buildkit/build/core-10674-4ayju/sites/default/civicrm.settings.php on line 464

@herbdool
Copy link
Contributor Author

@seamuslee001 thanks, I've attempted your request and fixed the syntax error.

@monishdeb
Copy link
Member

monishdeb commented Jul 17, 2017

@herbdool there's a false merge commits in the PR (see the 2nd commit). To avoid that, always rebase your current branch then rebase. Here are the steps to do that :

  1. cd [civicrm directory]
  2. Then rebase
git fetch upstream master
git pull --rebase upstream master
  1. Update your PR by git push -f herbdool crm20764

@herbdool
Copy link
Contributor Author

@monishdeb how about now?

@monishdeb
Copy link
Member

Perfect 😃👍

@mlutfy
Copy link
Member

mlutfy commented Nov 15, 2017

@monishdeb @herbdool Any reason why this hasn't been merged yet?

@herbdool
Copy link
Contributor Author

@mlutfy I'm not sure why it hasn't been merged. I think it's ready.

@seamuslee001
Copy link
Contributor

Code looks good to me

@mlutfy mlutfy merged commit a6bd202 into civicrm:master Nov 15, 2017
@mlutfy
Copy link
Member

mlutfy commented Nov 15, 2017

OK, taking the liberty of merging this one, based on the comments/reviews.

sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
CRM-20764 Allow clean URLs with CiviCRM on Backdrop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants