Replace a load of references to the wiki with docs links #17900

aydun · 2020-07-20T21:15:58Z

Overview

Change some code comments & help links from the old wiki to new docs.
Change docs link from 'stable' to 'latest' (avoids redirection)

civibot · 2020-07-20T21:16:01Z

(Standard links)

If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

eileenmcnaughton · 2020-07-20T21:28:57Z

@aydun looks like you need to negotiate a bit with jenkins

@MikeyMJCO can you review this?

homotechsual · 2020-07-20T21:41:04Z

Looks good to me!

colemanw · 2020-07-20T21:47:39Z

@aydun the style warnings were:

Job.php:59, EndLine, Priority: High
--
Whitespace found at end of line


Job.php:73, EndLine, Priority: High
--
Whitespace found at end of line


Job.php:73, EmptyLines, Priority: High
--
Functions must not contain multiple empty lines in a row; found 2 empty lines

eileenmcnaughton · 2020-07-20T21:49:42Z

I think with the @MikeyMJCO stamp of approval you just need the jenkins one & this is mergeable

eileenmcnaughton · 2020-07-20T21:50:18Z

ok-without-test as only comment blocks involved

homotechsual · 2020-07-20T21:55:23Z

There are non-comment changes - to the help link destinations/code.

seamuslee001 · 2020-07-20T22:13:06Z

CRM/Utils/Check/Component/Env.php

      $message->addHelp(
        ts('A default mailbox must be configured for email bounce processing.') . '<br />' .
-        ts("Learn more in the <a %1>online documentation</a>.", [1 => $docUrl])
+          CRM_Utils_System::docURL2('user/advanced-configuration/email-system-configuration/')


Are we loosing the original link text here?

Yes ... in some places we have things like this one: "Learn more in the online documentation" with a link, other places "See the XXX section in the documentation" with a link, others use the default rendering of "(Learn more ...)" with a link. I've adopted the more concise, default form.

So this one changes from:
"A default mailbox must be configured for email bounce processing. Learn more in the online documentation."
to
"A default mailbox must be configured for email bounce processing. (Learn more...)"

I prefer the more concise form.

However I can't see where the "Learn more..." text is being passed in here.

Nope, I see it - in the docUrl functions.

It's generated by docURL2()

seamuslee001 · 2020-07-20T22:15:06Z

CRM/Utils/Check/Component/Env.php

@@ -606,7 +605,7 @@ public function checkExtensions() {
        __FUNCTION__,
        ts('There are currently no extensions on the CiviCRM public extension directory which are compatible with version %1. If you want to install an extension which is not marked as compatible, you may be able to <a %2>download and install extensions manually</a> (depending on access to your web server).', [


I think this will need to be changed so its

Suggested change

ts('There are currently no extensions on the CiviCRM public extension directory which are compatible with version %1. If you want to install an extension which is not marked as compatible, you may be able to <a %2>download and install extensions manually</a> (depending on access to your web server).', [

ts('There are currently no extensions on the CiviCRM public extension directory which are compatible with version %1. If you want to install an extension which is not marked as compatible, you may be able to <a href="%2">download and install extensions manually</a> (depending on access to your web server).', [

I think the original is correct, or at least the civi standard, because it allows adding/removing/changing the attributes without breaking already translated strings. See https://docs.civicrm.org/dev/en/latest/translation/#avoid-tags-inside-strings where it talks about hyperlinks inside big text blocks.

I guess my thinking is that wit the proposed change the href= is getting droped in param 2 so it needs to either get restored there or be added here

Oh ok I see. Then it would be <a %2> and the href would be part of the array param 2 string same as before.

You're right, it's lost the href. Changed now to default form

seamuslee001 · 2020-07-20T22:16:02Z

templates/CRM/Admin/Form/Setting/Mail.tpl

 <div class="help">
-    {ts 1=$docLink}These settings are used to configure mailer properties for the optional CiviMail component and may allow you to significantly optimize performance. Please read the %1 documentation, and make sure you understand it before modifying default values. (These settings are NOT used for the built-in 'Email - send now' feature).{/ts}
+    {ts}These settings are used to configure mailer properties for the optional CiviMail component and may allow you to significantly optimize performance. Please read the documentation, and make sure you understand it before modifying default values. (These settings are NOT used for the built-in 'Email - send now' feature).{/ts} {docURL page="sysadmin/setup/civimail/outbound"}


should we not still be passing through the text="xxx"here?

See comment above

seamuslee001 · 2020-07-20T22:16:33Z

templates/CRM/Admin/Page/Options.hlp

  <p>{ts}Activities are 'interactions with contacts' which you want to record and track. CiviCRM has several reserved (e.g. 'built-in') activity types (meetings, phone calls, emails sent). Create additional 'activity types' here if you need to record other types of activities. For example, you might want to add 'Job Interview', or 'Site Audit', etc.{/ts}</p>
  {capture assign=crmURL}{crmURL p='civicrm/admin/custom/group' q='reset=1'}{/capture}
  <p>{ts 1=$crmURL}Subject, location, date/time and description fields are provided for all activity types. You can add custom fields for tracking additional information about activities <a href='%1'>here</a>.{/ts}</p>
-  <p>{ts 1=$docLink}Scheduled and Completed Activities are searchable by type and/or activity date using 'Advanced Search'. Other applications may record activities for CiviCRM contacts using our APIs. For more information, refer to the online %1.{/ts}</p>
+  <p>{ts}Scheduled and Completed Activities are searchable by type and/or activity date using 'Advanced Search'. Other applications may record activities for CiviCRM contacts using our APIs.{/ts} {docURL page="dev/api"} </p>


Again should we still be passing through the text here?

See comment above

seamuslee001 · 2020-07-20T22:18:16Z

I've added a few comments on where I think things could be improved

Change docs links to 'latest' not 'stable'

aydun · 2020-07-21T09:26:37Z

Fixed the style issues (although not on lines I originally changed). Changed CRM/Utils/Check/Component/Env.php and standardised a few more messages there.

colemanw · 2020-07-21T14:44:57Z

Looks like all the review comments have been addressed and jenkins is happy.

civibot bot added the master label Jul 20, 2020

eileenmcnaughton added the merge on pass label Jul 20, 2020

eileenmcnaughton added the ok-without-test label Jul 20, 2020

homotechsual approved these changes Jul 20, 2020

View reviewed changes

seamuslee001 reviewed Jul 20, 2020

View reviewed changes

Replace a load of references to the wiki with docs links

d05f355

Change docs links to 'latest' not 'stable'

aydun force-pushed the wiki_changes branch from 86f7691 to d05f355 Compare July 21, 2020 09:23

colemanw merged commit ff5ac94 into civicrm:master Jul 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace a load of references to the wiki with docs links #17900

Replace a load of references to the wiki with docs links #17900

aydun commented Jul 20, 2020

civibot bot commented Jul 20, 2020

eileenmcnaughton commented Jul 20, 2020

homotechsual commented Jul 20, 2020

colemanw commented Jul 20, 2020

eileenmcnaughton commented Jul 20, 2020

eileenmcnaughton commented Jul 20, 2020

homotechsual commented Jul 20, 2020

seamuslee001 Jul 20, 2020

aydun Jul 21, 2020

homotechsual Jul 21, 2020

homotechsual Jul 21, 2020

homotechsual Jul 21, 2020

aydun Jul 21, 2020

seamuslee001 Jul 20, 2020

demeritcowboy Jul 21, 2020

seamuslee001 Jul 21, 2020

demeritcowboy Jul 21, 2020 •

edited

Loading

aydun Jul 21, 2020

seamuslee001 Jul 20, 2020

aydun Jul 21, 2020

seamuslee001 Jul 20, 2020

aydun Jul 21, 2020

seamuslee001 commented Jul 20, 2020

aydun commented Jul 21, 2020

colemanw commented Jul 21, 2020

		@@ -606,7 +605,7 @@ public function checkExtensions() {
		__FUNCTION__,
		ts('There are currently no extensions on the CiviCRM public extension directory which are compatible with version %1. If you want to install an extension which is not marked as compatible, you may be able to <a %2>download and install extensions manually</a> (depending on access to your web server).', [

Replace a load of references to the wiki with docs links #17900

Replace a load of references to the wiki with docs links #17900

Conversation

aydun commented Jul 20, 2020

Overview

civibot bot commented Jul 20, 2020

eileenmcnaughton commented Jul 20, 2020

homotechsual commented Jul 20, 2020

colemanw commented Jul 20, 2020

eileenmcnaughton commented Jul 20, 2020

eileenmcnaughton commented Jul 20, 2020

homotechsual commented Jul 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

demeritcowboy Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seamuslee001 commented Jul 20, 2020

aydun commented Jul 21, 2020

colemanw commented Jul 21, 2020

demeritcowboy Jul 21, 2020 •

edited

Loading