-
Notifications
You must be signed in to change notification settings - Fork 123
[FLOSS H3] mozprefs: Fixing silent failing for mozprefs plugin #4772
[FLOSS H3] mozprefs: Fixing silent failing for mozprefs plugin #4772
Conversation
Doc news changes 2019 11 26
…19-11-26 Revert "Doc news changes 2019 11 26"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change!
Reading through the code all the changes seem appropriate to fix the mentioned issues.
Just run the format patches proposed by the continuous-integration/jenkins/pr-merge
check to fix the format.
I wonder why only one check was run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
Please fix the styling issues as reported by the CI (you can apply the patch provided by the CI), clean up the commit history (rebase!) and answer the questions of my review.
- <<TODO>> | ||
- <<TODO>> | ||
|
||
### mozprefs | ||
|
||
-Fixed bug when inserting Metakeys it would fail silently _(Nikola Prvulovic @Dynamichost96)_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-Fixed bug when inserting Metakeys it would fail silently _(Nikola Prvulovic @Dynamichost96)_ | |
-Fixed bug: When inserting unknown meta keys it failed silently _(Nikola Prvulovic @Dynamichost96)_ |
{ | ||
const Key * meta = ksAtCursor (metaKeys, jt); | ||
const char * pos = (const char *) keyName (meta); | ||
if (elektraStrNCmp (pos, "meta:/internal/mozprefs", 19) != 0 && elektraStrCmp (pos, "meta:/origname") && elektraStrNCmp (pos, "meta:/rename", 12) != 0 && elektraStrCmp (pos, "meta:/binary") != 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you only compare the first 19 characters in elektraStrNCmp (pos, "meta:/internal/mozprefs", 19)
?
Please use a consistent coding style (elektraStrCmp (pos, "meta:/origname")
was not explicitly compared against 0
).
if (elektraStrNCmp (pos, "meta:/internal/mozprefs", 19) != 0 && elektraStrCmp (pos, "meta:/origname") && elektraStrNCmp (pos, "meta:/rename", 12) != 0 && elektraStrCmp (pos, "meta:/binary") != 0) | |
if (elektraStrNCmp (pos, "meta:/internal/mozprefs", 23) != 0 && elektraStrCmp (pos, "meta:/origname") != 0 && elektraStrNCmp (pos, "meta:/rename", 12) != 0 && elektraStrCmp (pos, "meta:/binary") != 0) |
I mark this stale as it did not have any activity for one year. I'll close it in two weeks if no further activity occurs. If you want it to be alive again, ping by writing a message here or create a new PR with the remainder of this PR. |
I closed this now because it has been inactive for more than one year. If I closed it by mistake, please do not hesitate to reopen it or create a new PR with the remainder of this PR. |
Fixes #4585
Basics
(added as entry in
doc/news/_preparation_next_release.md
whichcontains
_(my name)_
)Please always add something to the release notes.
(first line should have
module: short statement
syntax)close #X
, are in the commit messages.doc/news/_preparation_next_release.md
scripts/dev/reformat-all
Checklist
(not in the PR description)
Review
Labels