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

Spelling mistake "separator" not "seperator" #18238

Merged
merged 1 commit into from
Aug 23, 2020

Conversation

agileware-justin
Copy link
Contributor

Overview

This PR fixes a spelling mistake "separator" not "seperator" which inconsistent throughout the code.

Before

Sometimes variables, comments etc. use "seperator" and sometimes "separator" which could cause future bugs due to the incorrect spelling.

After

Fixed spelling and "separator" is now used consistently.

Technical Details

Have reviewed the changes and does not look like it should break anything.

Looks like the change to templates/CRM/Event/Form/ParticipantFeeSelection.tpl may actually fix a bug due to the incorrect variable name.
Where "seperator" is referenced here, https://github.com/civicrm/civicrm-core/blob/master/templates/CRM/Event/Form/ParticipantFeeSelection.tpl#L21
With the "separator" defined here, https://github.com/civicrm/civicrm-core/blob/master/templates/CRM/Price/Form/Calculate.tpl#L28

Comments

Fingers crossed unit tests catch any problems with this change.

@civibot
Copy link

civibot bot commented Aug 23, 2020

(Standard links)

@civibot civibot bot added the master label Aug 23, 2020
@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS but the event part suggests a related problem
      • For the fee selection one, obviously the spelling change is correct. But I did the below steps and I think there's something else weird on this page because there's two javascript display() functions defined, one in ParticipantFeeSelection.tpl and one in Calculate.tpl. The one that gets triggered is the one in Calculate which doesn't use seperator/separator. I'm not sure where the changed function is used.
        • Find Participants.
        • Click on view for one of them.
        • Click on the blue link Change Selections.
        • Change the fee selection.
        • No difference before or after patch. No errors in console.
      • I didn't run everything but the rest looks correct and grepping for it and variations there's just a handful of other comment lines.
  • Developer standards
    • [r-tech] PASS
      • I remember for the longest time the similar DAO constant that was used everywhere was also a typo. Something about this word. I wonder if there's an easy way to add a rule to civilint.
    • [r-code] PASS
    • [r-maint] ?
    • [r-test] PASS

@seamuslee001
Copy link
Contributor

I'm going to merge this based on @demeritcowboy 's review here It might be useful @demeritcowboy if you put up that issue around the js files in a lab ticket for someone to work on in the future

@seamuslee001 seamuslee001 merged commit 5b88eb7 into civicrm:master Aug 23, 2020
@demeritcowboy
Copy link
Contributor

Created https://lab.civicrm.org/dev/event/-/issues/41. Seems the display() function is actually defined 3 times - maybe it just needs to define it one more time to display something...

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.

3 participants