Codechecker fixes #759
Merged
Codechecker fixes #759
Conversation
This file contains 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
danmarsden
reviewed
Jul 8, 2024
| * @param string $encoding The encoding of the csv file. | ||
| * @param string $delimiter The specified delimiter for the file. | ||
| * @param string $importid The id of the csv import. | ||
| * @param array $mappingdata The mapping data from the import form. | ||
| * @param bool $useprogressbar Whether progress bar should be displayed, to avoid html output on CLI. | ||
| */ | ||
| public function __construct($text = null, $att, $encoding = null, $delimiter = null, $importid = 0, | ||
| public function __construct($att, $text = null, $encoding = null, $delimiter = null, $importid = 0, |
There was a problem hiding this comment.
did you change the order of these params on purpose? - this doen'st seem to be a codechecker change?
danmarsden
reviewed
Jul 8, 2024
| $form = null; | ||
| if (optional_param('needsconfirm', 0, PARAM_BOOL)) { | ||
| $form = new \mod_attendance\form\import\marksessions($url->out(false), $formparams); | ||
| } else if (optional_param('confirm', 0, PARAM_BOOL)) { | ||
| $importer = new \mod_attendance\import\marksessions(null, $att, null, null, $importid); |
There was a problem hiding this comment.
looks like here'e the matching change for the function parameter order change - I think we should probably do this as it's own commit rather than bundling into the "codechecker fixes" if there's a good reason we should do this.
lucaboesch
force-pushed
the
codecheckerfixes
branch
from
a4f124a
to
f9528d3
Compare
Realized the order of the paramaters so that no mandatory follows an optional one.
lucaboesch
force-pushed
the
codecheckerfixes
branch
from
f9528d3
to
c1d0325
Compare
|
Well spotted, @danmarsden. Best, |
|
cool thanks! - merged it in. |
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.
Adhere to more recent Moodle Coding Guidelines.