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.

Sign up for GitHub

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

Jump to bottom

Added ServiceNow Sync package #7155

Closed
wants to merge 1 commit into from
Closed

Added ServiceNow Sync package #7155

wants to merge 1 commit into from

Conversation

salcosta
Copy link

No description provided.

packagecontrol-bot
packagecontrol-bot approved these changes Jun 15, 2018
View reviewed changes
Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: SUCCESS

Repo link: ServiceNow Sync

Packages added:
  - ServiceNow Sync

Processing package "ServiceNow Sync"
  - All checks passed

@Thom1729
Copy link
Collaborator

  • You shouldn't modify sys.path, because that is shared between all packages.
  • You have an empty __init__.py.
  • Your readme doesn't describe what the package does or who would use it.

@salcosta
Copy link
Author

salcosta commented Jul 2, 2018

@Thom1729 Appending the current path to the sys.path seems to be the standard across other packages. Is there an alternative you are wanting to use now?

Regarding the other 2 changes I will resolve those.

@Thom1729
Copy link
Collaborator

Thom1729 commented Jul 4, 2018

Instead of modifying sys.path, use relative imports: from .Commands.create_connection import *.

@Thom1729
Copy link
Collaborator

@salcosta ping.

@Thom1729 Thom1729 added the stale The pull request needs to be updated but has not been within the recent past (2 weeks) label Aug 31, 2018
@FichteFoll FichteFoll added timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale) and removed stale The pull request needs to be updated but has not been within the recent past (2 weeks) labels Dec 31, 2018
@FichteFoll FichteFoll closed this Dec 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@packagecontrol-bot packagecontrol-bot packagecontrol-bot approved these changes

Assignees
No one assigned
Labels
timeout A pull request needed changes but was not updated in time (2 weeks after becoming stale)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@salcosta @Thom1729 @packagecontrol-bot @FichteFoll