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

Updated Trac and GitHub issue references #470

Merged
merged 14 commits into from
Jun 7, 2017
Merged

Updated Trac and GitHub issue references #470

merged 14 commits into from
Jun 7, 2017

Conversation

mlewand
Copy link
Contributor

@mlewand mlewand commented Jun 6, 2017

What is the purpose of this pull request?

Maintenance Task

What changes did you make?

Replaced existing #<number> references with permalinks to Trac. Also updated tags in manual tests, from format <number> to trac<number>.

GitHub references has been updated from gh<number> to <number> in manual tests, and #<number> in js files.

Closes #433.

This issue does not require changelog entry.

@Comandeer Comandeer self-requested a review June 6, 2017 11:15
core/editable.js Outdated
if ( this.insertElementIntoRange( element, range ) ) {
range.moveToPosition( element, CKEDITOR.POSITION_AFTER_END );

// If we're inserting a block element, the new cursor position must be
// optimized. (#3100,#5436,#8950)
// optimized. (http://dev.ckeditor.com/ticket/3100,#5436,#8950)
Copy link
Member

Choose a reason for hiding this comment

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

Not all tickets were changed in this line.

core/editable.js Outdated
@@ -1172,7 +1172,7 @@
}

// Prevent Webkit/Blink from going rogue when joining
// blocks on BACKSPACE/DEL (#11861,#9998).
// blocks on BACKSPACE/DEL (http://dev.ckeditor.com/ticket/11861,#9998).
Copy link
Member

Choose a reason for hiding this comment

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

Not all tickets were changed in this line.

@@ -192,7 +192,7 @@
data.writeChildrenHtml( writer );
data = writer.getHtml( true );

// Restore those non-HTML protected source. (#4475,#4880)
// Restore those non-HTML protected source. (http://dev.ckeditor.com/ticket/4475,#4880)
Copy link
Member

Choose a reason for hiding this comment

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

Not all tickets were changed in this line.

return isUnstylable( element );
} : function( element ) {
// Fore color style must be applied inside links instead of around it. (#4772,#6908)
// Fore color style must be applied inside links instead of around it. (http://dev.ckeditor.com/ticket/4772,#6908)
Copy link
Member

Choose a reason for hiding this comment

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

Not all tickets were changed in this line.

@@ -10,7 +10,7 @@ CKEDITOR.plugins.add( 'devtools', {
editor._.showDialogDefinitionTooltips = 1;
},
onLoad: function() {
CKEDITOR.document.appendStyleText( CKEDITOR.config.devtools_styles || '#cke_tooltip { padding: 5px; border: 2px solid #333; background: #ffffff }' +
CKEDITOR.document.appendStyleText( CKEDITOR.config.devtools_styles || '#cke_tooltip { padding: 5px; border: 2px solid http://dev.ckeditor.com/ticket/333; background: #ffffff }' +
Copy link
Member

Choose a reason for hiding this comment

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

It's not a ticket number.

@@ -1,5 +1,5 @@
@bender-ui: collapsed
@bender-tags: tc, 16755, 4.7.0, tp1579
@bender-tags: tc, 16755, 4.7.0, tptrac1579
Copy link
Member

Choose a reason for hiding this comment

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

It's tp ticket number, not trac. Additionally trac ticket is not changed.

@@ -1,4 +1,4 @@
@bender-tags: 4.6.1, tc, 11064, widgetselection
@bender-tags: 4.6.1, tc, 1trac1064, widgetselection
Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't be changed this way.

@@ -1,4 +1,4 @@
@bender-tags: 4.6.1, tc, 11064, widgetselection
@bender-tags: 4.6.1, tc, 1trac1064, widgetselection
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be changed this way.

@@ -1,5 +1,5 @@
@bender-ui: collapsed
@bender-tags: wyswigarea, tc, 4.5.2, 11616, 9715
@bender-tags: wyswigarea, tc, 4.5.2, 1trac1616, 9715
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be changed this way.

@@ -1,5 +1,5 @@
@bender-ui: collapsed
@bender-tags: toolbar, tc, 4.5.2, 11616, 7360
@bender-tags: toolbar, tc, 4.5.2, 1trac1616, 7360
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be changed this way.

@mlewand
Copy link
Contributor Author

mlewand commented Jun 6, 2017

Added requested changes.

core/editable.js Outdated
@@ -998,7 +998,7 @@
var keyCode = evt.data.domEvent.getKey(),
isHandled;

// Prevent of reading path of empty range (#13096, #gh457).
// Prevent of reading path of empty range (http://dev.ckeditor.com/ticket/13096, http://dev.ckeditor.com/ticket/457).
Copy link
Member

Choose a reason for hiding this comment

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

GH ticket number is now changed into trac link.

core/editable.js Outdated
@@ -1186,7 +1186,7 @@
if ( !( key in backspaceOrDelete ) )
return;

// Prevent of reading path of empty range (#13096, #gh457).
// Prevent of reading path of empty range (http://dev.ckeditor.com/ticket/13096, http://dev.ckeditor.com/ticket/457).
Copy link
Member

Choose a reason for hiding this comment

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

GH ticket number is now changed into trac link.

@@ -1,4 +1,4 @@
@bender-tags: selection, focus, tc, 4.4.6, 12630, 12337
@bender-tags: selection, focus, tc, 4.4.6, 12630, trac12337
Copy link
Member

Choose a reason for hiding this comment

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

There is still second ticket number, which was not changed.

@@ -1,4 +1,4 @@
@bender-tags: selection, focus, tc, 4.4.6, 12630, 12337
@bender-tags: selection, focus, tc, 4.4.6, 12630, trac12337
Copy link
Member

Choose a reason for hiding this comment

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

There is still second ticket number, which was not changed.

@@ -1,4 +1,4 @@
@bender-tags: selection, focus, tc, 4.4.6, 12630, 12337
@bender-tags: selection, focus, tc, 4.4.6, 12630, trac12337
Copy link
Member

Choose a reason for hiding this comment

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

There is still second ticket number, which was not changed.

@@ -20,7 +20,7 @@
}

var tests = {
// (#258, #tp2247)
// (http://dev.ckeditor.com/ticket/258, #tp2247)
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this ticket number is from unknown source… It does not fit to trac or our GH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this issue is not available to the public, I decided to remove it all along.

@mlewand
Copy link
Contributor Author

mlewand commented Jun 7, 2017

I found a couple extra missing references, fixed in 3d04295.

@Comandeer Comandeer self-requested a review June 7, 2017 12:26
@@ -1,4 +1,4 @@
@bender-tags: selection, focus, tc, 4.4.6, 12630, 12337
@bender-tags: selection, focus, tc, 4.4.6, 12630, trac12337
Copy link
Member

Choose a reason for hiding this comment

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

Still one ticket number left.

@mlewand
Copy link
Contributor Author

mlewand commented Jun 7, 2017

Fixed. I did not rebase the branch, so that comments won't lose the reference.

@Comandeer Comandeer self-requested a review June 7, 2017 13:50
Copy link
Member

@Comandeer Comandeer left a comment

Choose a reason for hiding this comment

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

LGTM. But it would be nice to rebase the branch and fix also the newly added ticket numbers.

@mlewand
Copy link
Contributor Author

mlewand commented Jun 7, 2017

and fix also the newly added ticket numbers

It's a relatively recent PR, and it's latest common commit with master is 83a5ae1.

But it would be nice to rebase the branch

Done.

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.

2 participants