-
Notifications
You must be signed in to change notification settings - Fork 2
Misc/codecleaning #18
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
Open
TamaroWalter
wants to merge
20
commits into
main
Choose a base branch
from
misc/codecleaning
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add missing docblocks and format comments like moodle wants them.
Fix missing or irrelevant moodle internal checks. Replace include with require_once statements
Move the $CFG call from the language file to the lib.php where all other params for the string are build.
Replace deprecated print_error() with throw new moodle_exception()
Reformat lines that are longer than 132 characters by using helper variables or spreading a statement over multiple lines
Replace long strings with plugin language string
This function is particularly long and should be reviewed separatedly due to a higher risk of errors
Another particularly long lines that are separated from other commits for the code review
The CI wants the tables to have the full "local_lsf_unification" prefix
24ef2ce to
c9818b6
Compare
c9818b6 to
2c0ec5b
Compare
Member
Author
|
Until this point the code is technically fine. Now the businesslogic fails, which will be fixed in the next commits |
The original email body was not suitable for testing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔀 Purpose of this PR:
📝 Description:
This PR fixes all current phpcs errors. The commits are separated for review and can/should be rebased to one "code cleaning" commit before merging.
📋 Checklist
Please confirm the following (check all that apply):
phpunitand/orbehattests that cover my changes or additions.var_dump()orvar_exportor any other debugging statements (or commented out code) thatshould not appear on the productive branch.
db/upgrade.phpandupdated the
version.php..minfiles with thegrunt amdcommand.version.phpand theCHANGES.md.I ran all tests thoroughly checking for errors. I checked if bootstrap had any changes/deprecations that require
changes in the plugins UI.