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.

Sign up for GitHub

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

Jump to bottom

[Timepoint/Create Timepoint] Add timepoint language feature #4976

Merged
merged 18 commits into from
Feb 26, 2020
Merged

[Timepoint/Create Timepoint] Add timepoint language feature #4976

merged 18 commits into from
Feb 26, 2020

Conversation

jesscall
Copy link
Contributor

Brief summary of changes

This PR adds language feature to timepoints.

Testing instructions (if applicable)

  1. Run patches in this PR + make sure you have changes that were merged yesterday from [SQL] Rename project_rel table and add constraints #4492
  2. Create a new timepoint and choose a language.
  3. You can add french test names to test_names_fr so the instrument list is translated according to the language of the session.

@jesscall jesscall added Category: Feature PR or issue that aims to introduce a new feature Language: SQL PR or issue that update SQL code [branch] minor labels Jul 23, 2019
xlecours
xlecours requested changes Jul 23, 2019
View reviewed changes
Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

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

I think this could easily support more than just French.

SQL/0000-00-00-schema.sql Outdated Show resolved Hide resolved
SQL/0000-00-00-schema.sql Outdated Show resolved Hide resolved
@johnsaigle johnsaigle added the State: Needs work PR awaiting additional work by the author to proceed label Jul 23, 2019
@johnsaigle
Copy link
Contributor 

FILE: /home/travis/build/aces/Loris/php/libraries/TimePoint.class.inc
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 188 | ERROR | Doc comment for parameter "$lang" missing
----------------------------------------------------------------------

PHPCS issue

@jesscall jesscall removed the State: Needs work PR awaiting additional work by the author to proceed label Aug 1, 2019
xlecours
xlecours requested changes Aug 2, 2019
View reviewed changes
Copy link
Contributor

@xlecours xlecours left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I haven't tested it but it looks great!

I specially like the fact that you do not use language_id which is meaningless anyway.

  • small change request for $lang param

php/libraries/TimePoint.class.inc Outdated Show resolved Hide resolved
@johnsaigle johnsaigle requested a review from xlecours August 19, 2019 19:29
@jesscall jesscall added State: Needs work PR awaiting additional work by the author to proceed State: Needs RB update PR that needs to update Raisinbread to reflect its changes labels Aug 23, 2019
maltheism
maltheism previously requested changes Sep 30, 2019
View reviewed changes
modules/instrument_list/php/instrument_list.class.inc Outdated Show resolved Hide resolved
@ridz1208 ridz1208 added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 6, 2019
@ridz1208 ridz1208 changed the base branch from minor to master December 13, 2019 19:20
@maltheism
Copy link
Member

@jesscall I was assigned to review this PR. I'm just leaving a comment to get a notification when the conflicts are resolved.

jesscall and others added 3 commits January 13, 2020 14:39
added session language feature
844033b 
touch ups

revamp to allow for multiple languages

phpcs

fixed function signature and added default

travis

added some rb thangs

amendments

just trying to get rb to reset

[addign langauge feature] starting to extend the changes in code references
revision
e1eab4f
validation
160279a
@jesscall jesscall force-pushed the 2019_07_05_Add_Language_feature branch from 4c2fb2b to 160279a Compare January 15, 2020 20:11
@jesscall jesscall removed State: Needs RB update PR that needs to update Raisinbread to reflect its changes State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) State: Needs work PR awaiting additional work by the author to proceed labels Jan 15, 2020
xlecours
xlecours requested changes Jan 21, 2020
View reviewed changes
php/libraries/TimePoint.class.inc Outdated Show resolved Hide resolved
@jesscall
Copy link
Contributor Author

Just having some trouble with integration tests... travis is saying it cannot find element psc.
Maybe someone with more experience has some insight on how to fix this?

@ridz1208
Copy link
Collaborator

@johnsaigle is my review even meaningful if @xlecours already approved it??

i think not 🙇‍♂️

@johnsaigle johnsaigle dismissed ridz1208’s stale review January 27, 2020 18:48

Dismissing because Rida claims he is unworthy

driusan
driusan previously requested changes Jan 28, 2020
View reviewed changes
SQL/0000-00-00-schema.sql Outdated Show resolved Hide resolved
SQL/New_patches/2019_07_05_Add_Language_feature.sql Outdated Show resolved Hide resolved
modules/create_timepoint/php/create_timepoint.class.inc Outdated Show resolved Hide resolved
@driusan driusan added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Feb 25, 2020
@driusan driusan self-requested a review February 25, 2020 15:47
@driusan driusan dismissed their stale review February 25, 2020 15:48

need to rereview

@driusan
Copy link
Collaborator

driusan commented Feb 25, 2020

@jesscall this has raisinbread conflicts

@jesscall jesscall removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Feb 25, 2020
@jesscall
Copy link
Contributor Author

@driusan conflicts resolved

@driusan driusan merged commit 2161c9f into aces:master Feb 26, 2020
@ridz1208
Copy link
Collaborator

🎉

@ridz1208 ridz1208 added this to the 23.0.0 milestone Feb 27, 2020
@ridz1208 ridz1208 added the Release: Add to release notes PR whose changes should be highlighted in the release notes label Feb 27, 2020
@driusan driusan mentioned this pull request Mar 9, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xlecours xlecours xlecours approved these changes

@johnsaigle johnsaigle johnsaigle left review comments

@maltheism maltheism maltheism approved these changes

@ridz1208 ridz1208 Awaiting requested review from ridz1208

@driusan driusan Awaiting requested review from driusan

Assignees

@jesscall jesscall

Labels
Category: Feature PR or issue that aims to introduce a new feature Language: SQL PR or issue that update SQL code Release: Add to release notes PR whose changes should be highlighted in the release notes
Projects
None yet
Milestone
23.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jesscall @johnsaigle @maltheism @ridz1208 @driusan @xlecours