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

Begin implementing the pr-only option. #52

Merged
merged 7 commits into from
Aug 9, 2017
Merged

Conversation

allenh1
Copy link
Collaborator

@allenh1 allenh1 commented Aug 2, 2017

connects to #49

@allenh1 allenh1 force-pushed the add-pr-only-option branch 3 times, most recently from b325653 to ae0c5a1 Compare August 7, 2017 22:04
@allenh1 allenh1 requested a review from tfoote August 7, 2017 23:18
@allenh1
Copy link
Collaborator Author

allenh1 commented Aug 7, 2017

I think I'll catch the OpenEmbedded args up at a later date.


Next step: add a configureable repo-directory option.

@@ -19,14 +19,14 @@ def get_random_branch_name():


class ros_overlay(repo_instance):
def __init__(self):
def __init__(self, existing_repo=None):
Copy link
Member

Choose a reason for hiding this comment

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

Is this new argument used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would appear not -- good catch.

@@ -19,7 +19,7 @@


class repo_instance(object):
Copy link
Member

Choose a reason for hiding this comment

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

This class should be RepoInstance

@@ -19,14 +19,14 @@ def get_random_branch_name():


class ros_overlay(repo_instance):
Copy link
Member

Choose a reason for hiding this comment

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

This class should be CamelCased as well.

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the initialization. Do you really need the inheritance here or can you just have the repo_instance as a member?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point.

I think I should also move the pull_request function into RepoInstance

@@ -19,14 +19,14 @@ def get_random_branch_name():


class ros_overlay(repo_instance):
def __init__(self):
def __init__(self, existing_repo=None):
# clone repo into a random tmp directory.
repo_instance.__init__(self, 'ros',
Copy link
Member

Choose a reason for hiding this comment

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

Use super to init the base class. lots of details here: https://stackoverflow.com/questions/576169/understanding-python-super-with-init-methods

@@ -13,7 +13,8 @@
# limitations under the License.


from superflore import repo_instance
# TODO(allenh1): remove inheritance like the RosOverlay class.
from superflore import RepoInstance as repo_instance
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a temporary hacky-fix to avoid breaking things in the OpenEmbedded world. This will be updated when I catch OpenEmbedded up.

@allenh1
Copy link
Collaborator Author

allenh1 commented Aug 9, 2017

@tfoote rebased on top of master. Ready for review.

@allenh1 allenh1 merged commit 6931052 into master Aug 9, 2017
@allenh1 allenh1 deleted the add-pr-only-option branch August 9, 2017 20:00
@allenh1 allenh1 removed the in review label Aug 9, 2017
allenh1 added a commit that referenced this pull request Oct 31, 2017
allenh1 added a commit that referenced this pull request Nov 2, 2017
* Switch to use util functions.

* Use the generate_installers function from superflore.generate_installers

* Linting

* Change the ros_meta class to be RosMeta, as well as changed the inheritance
to be a member of the class (see #52).

* Removed the sym-linked structure to behave more like the Gentoo generator (see #51).

* Removed a commented line from the ebuild logic copypasta, changed the print format slightly for the exception output after a failed pr, and called the file_pr function.

* Add temporary file manager code, as suggested by @tfoote.

* Switch to utils print functions for TempfileManager output.

* Use tempfile manager to clean up temp directories at exit.

* Linting.

* Remove hard-coded org and repo from RepoInstance call.

* Reorder import statements.

* Ok, that's pretty cool.

* Apply new TempfileManager usage to OpenEmbedded logic.

* OpenEmbedded generation is now running. ToDo: automatically clean up the tar files.

* Files were not generating in the directory, but are now.

* Fixed argument mapping for ebuild generator.
zffgithub pushed a commit to zffgithub/superflore that referenced this pull request Apr 11, 2023
* Switch to use util functions.

* Use the generate_installers function from superflore.generate_installers

* Linting

* Change the ros_meta class to be RosMeta, as well as changed the inheritance
to be a member of the class (see ros-infrastructure#52).

* Removed the sym-linked structure to behave more like the Gentoo generator (see ros-infrastructure#51).

* Removed a commented line from the ebuild logic copypasta, changed the print format slightly for the exception output after a failed pr, and called the file_pr function.

* Add temporary file manager code, as suggested by @tfoote.

* Switch to utils print functions for TempfileManager output.

* Use tempfile manager to clean up temp directories at exit.

* Linting.

* Remove hard-coded org and repo from RepoInstance call.

* Reorder import statements.

* Ok, that's pretty cool.

* Apply new TempfileManager usage to OpenEmbedded logic.

* OpenEmbedded generation is now running. ToDo: automatically clean up the tar files.

* Files were not generating in the directory, but are now.

* Fixed argument mapping for ebuild generator.
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