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

Hide SITS Grade push settings for users with no permission #25

Merged
merged 5 commits into from
Nov 3, 2023

Conversation

nbozhkov-ucl
Copy link
Contributor

No description provided.

@aspark21
Copy link
Collaborator

aspark21 commented Nov 2, 2023

Hi @nbozhkov-ucl

  1. Can you look at the coding standards feedback - https://github.com/ucl-isd/moodle-local_sitsgradepush/actions/runs/6733510917/job/18302374802#step:10:18

  2. make sure to deploy this branch on a preview instance for the reviewer to test\

Thanks
Al

@aydevworks
Copy link
Collaborator

I think no need to do step 1, because I have many changes to the codes for my current tickets. I will do the coding standards fix.

@aydevworks
Copy link
Collaborator

@nbozhkov-ucl I meant please just make sure your code changes are meeting the coding standards.

@nbozhkov-ucl
Copy link
Contributor Author

I think no need to do step 1, because I have many changes to the codes for my current tickets. I will do the coding standards fix.

Thanks Alex. I know the change I made is pretty simple but helps me start to learn Moodle coding. You can incorporate in your branch and close my pull request when needed.

@aspark21
Copy link
Collaborator

aspark21 commented Nov 2, 2023

Yep I meant 1 specifically for the lines you were changing, not about fixing the wider codebase errors.
2 still worth doing and this should be merged independently from other changes 😉

@nbozhkov-ucl
Copy link
Contributor Author

Code is deployed to 42-clc and all coding standards errors/warnings introduced by me fixed.

@aspark21
Copy link
Collaborator

aspark21 commented Nov 3, 2023

Tested - 👍

Code - looks good to me, thanks for fixing coding standards 😄

Copy link
Collaborator

@aydevworks aydevworks left a comment

Choose a reason for hiding this comment

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

Looks good.

@aspark21 aspark21 merged commit a42dd91 into main Nov 3, 2023
0 of 8 checks passed
@aspark21 aspark21 deleted the CTP-2760 branch November 3, 2023 10:39
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