Skip to content
This repository has been archived by the owner on Oct 10, 2020. It is now read-only.

Documenting some functions and methods that are difficult to understand. #1225

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

scovl
Copy link

@scovl scovl commented Apr 15, 2018

Description

Documenting some functions and methods that are difficult to understand. I feel that the atomic project codes lack some lines of comments to facilitate future contributions. I'm working on it.

Pull Request Checklist:

If your Pull request contains new features or functions, tests are required. If the PR is a bug fix and no tests exist, please consider adding some to prevent regressions.

  • Unittests
  • Integration Tests

Atomic/atomic.py Outdated
@@ -118,6 +124,11 @@ def pull(self):
prevstatus = status
util.write_out("")

# If for some reason the NameError or AtributeError class appears in the path,
# the exception is thrown to ignore the "pass" problem.
# The NameError class handles only unqualified names while AtributeError,
Copy link
Collaborator

Choose a reason for hiding this comment

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

s|AtributeError|AttributeError|

Copy link

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

@lobocode thanks for the PR, looks good in general, just a couple of small nits.

@@ -13,7 +13,13 @@
from docker.errors import NotFound
from .discovery import RegistryInspect, RegistryInspectError


Choose a reason for hiding this comment

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

I'd remove at least one if not two of these added blank lines.

Atomic/atomic.py Outdated
def find_repo_tag(d, Id, image_name):

# The image_in_repotags function fetches the name of the image inside the "repotag" repository, and if it finds,

Choose a reason for hiding this comment

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

suggest "if finds," to "found,"

@scovl
Copy link
Author

scovl commented Apr 16, 2018

I will make the considerations and add some other necessary comments. Thank's!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants