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

Documentation and Refactoring #3

Open
wants to merge 64 commits into
base: main
Choose a base branch
from
Open

Documentation and Refactoring #3

wants to merge 64 commits into from

Conversation

MacOS
Copy link

@MacOS MacOS commented Oct 15, 2021

This pull request aims at improving the code quality, which is achieved by

  1. adding preliminary documentation, and
  2. refactoring the code with respect to code quality, e.g. indentation and deletion of unnecessary variables declarations.

ordo/ordo.js Outdated
var toggleOpenButton = function(cell) {
<<<<<<< HEAD

Choose a reason for hiding this comment

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

Pls remove the merge artifacts, first!

Copy link
Author

Choose a reason for hiding this comment

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

done

ordo/ordo.js Outdated
if (cell.metadata.ordo !== undefined &&
cell.metadata.ordo.admonition !== undefined &&
cell.metadata.ordo.admonition) {
=======

Choose a reason for hiding this comment

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

Pls remove the merge artifacts, first!

Copy link
Author

Choose a reason for hiding this comment

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

done

ordo/ordo.js Outdated
@@ -214,67 +244,148 @@ define([
cell.element.find('div.ordo-admonition-controls').remove();
}
}
>>>>>>> 0c1a4b94feb516d3f1f0a5e4f8cda3e37fe3e17f

Choose a reason for hiding this comment

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

Pls remove the merge artifacts, first!

Copy link
Author

Choose a reason for hiding this comment

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

done

ordo/ordo.js Outdated


/**
* Toggels the admonition div between the states open and close

Choose a reason for hiding this comment

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

Toggels -> Toggles?

Copy link
Author

Choose a reason for hiding this comment

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

done

ordo/ordo.js Outdated
var initializeCells = function() {
<<<<<<< HEAD

Choose a reason for hiding this comment

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

Pls. remove the merge artifacts first.

Copy link
Author

Choose a reason for hiding this comment

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

done

ordo/ordo.js Outdated
var onEditSol = function(cell) {
<<<<<<< HEAD

Choose a reason for hiding this comment

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

Pls. remove the merge artifacts first.

ordo/ordo.js Outdated
'keyboard_manager': Jupyter.notebook.keyboard_manager,
'notebook': Jupyter.notebook
});
=======

Choose a reason for hiding this comment

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

Pls. remove the merge artifacts first.

Copy link
Author

Choose a reason for hiding this comment

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

done.

ordo/ordo.js Outdated
@@ -815,81 +1103,244 @@ define([
'keyboard_manager': Jupyter.notebook.keyboard_manager,
'notebook': Jupyter.notebook
})
>>>>>>> 0c1a4b94feb516d3f1f0a5e4f8cda3e37fe3e17f

Choose a reason for hiding this comment

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

Pls. remove the merge artifacts first.

Copy link
Author

Choose a reason for hiding this comment

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

done.

ordo/ordo.js Outdated
.then(initialize)
.catch(function on_error(reason) {
console.error('Error:', reason);
=======

Choose a reason for hiding this comment

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

Pls. remove the merge artifacts first.

Copy link
Author

Choose a reason for hiding this comment

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

done.

ordo/ordo.js Outdated
@@ -1038,9 +1489,11 @@ define([
var ordo_exts = function() {
return Jupyter.notebook.config.loaded.then(readConfig).then(initialize).catch(function on_error (reason) {
console.error('Error:', reason);
>>>>>>> 0c1a4b94feb516d3f1f0a5e4f8cda3e37fe3e17f

Choose a reason for hiding this comment

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

Pls. remove the merge artifacts first.

Copy link
Author

Choose a reason for hiding this comment

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

done.

@MacOS
Copy link
Author

MacOS commented Oct 20, 2021

I think the pull request has still an issue because the defering mechanisms in initialilzeCells is gone. In fact, I think that the function can be made simpler by deleting the first loop and only using the event as they do the same. But fixing this might be for the next pull request.

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