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

Added mod/checklist master 4.1.0.2 #88

Conversation

lbailey-ucsf
Copy link
Collaborator

The UAT team needs to review this PR before getting the go-ahead to merge. It's currently failing the following behat scenario in my dev environment:

001 Scenario: A teacher can link to a course or a URL, but not both # /var/www/html/mod/checklist/tests/behat/course_link.feature:33
      And the "linkcourseid" "field" should be disabled             # /var/www/html/mod/checklist/tests/behat/course_link.feature:38
        The attribute "disabled" should be set but is not

@lbailey-ucsf lbailey-ucsf marked this pull request as ready for review October 23, 2024 18:45
@mirleu
Copy link

mirleu commented Oct 24, 2024

@ctam @lbailey-ucsf in this case the .gitmodules file has new updates not included in this request. Should I request changes or approve and then Leon to fix the merge conflict?

@lbailey-ucsf
Copy link
Collaborator Author

@ctam @lbailey-ucsf in this case the .gitmodules file has new updates not included in this request. Should I request changes or approve and then Leon to fix the merge conflict?

I vote the latter. See what carson says.

@ctam
Copy link

ctam commented Oct 25, 2024

Ha! This is a good one. @mirleu, how do you know there will be a merge conflict already? Did you check it in the command line?
Normally, I think it should go back to the developer to do a git rebase and resolve all the conflicts, especially if the code change is complex. But in this case, it's just a couple of line changes in the .gitmodules file, so I don't think it matters. But if you're asking for some policy, yeah, it should go back to the developer before approving it. 🙂

Copy link

@mirleu mirleu left a comment

Choose a reason for hiding this comment

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

lgtm

@mirleu mirleu merged commit 2f426c8 into ucsf-education:UCSFCLE_404_STABLE Oct 25, 2024
@mirleu
Copy link

mirleu commented Oct 25, 2024

Ha! This is a good one. @mirleu, how do you know there will be a merge conflict already? Did you check it in the command line? Normally, I think it should go back to the developer to do a git rebase and resolve all the conflicts, especially if the code change is complex. But in this case, it's just a couple of line changes in the .gitmodules file, so I don't think it matters. But if you're asking for some policy, yeah, it should go back to the developer before approving it. 🙂

Turns out there was no merge conflict after I approved so I merged. I realized I was thinking of the M4.3 .gitmodules file but this is for M4.4. Good to know the process for next time. Thanks everyone!

@lbailey-ucsf lbailey-ucsf deleted the T-1548/UCSFCLE_404/add-mod_checklist branch October 25, 2024 15:51
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.

3 participants