Skip to content

Conversation

@metin-arm
Copy link
Contributor

@metin-arm metin-arm commented Feb 23, 2024

Commits:

  • 24a084e - target: Address pylint issues in ChromeOsTarget class
  • 1bd22c4 - target: tests: Add support for testing ChromeOS targets

devlib/target.py Outdated
Comment on lines 3016 to 3025
platform=platform,
working_directory=working_directory,
executables_directory=executables_directory,
connect=False,
modules=modules,
load_default_modules=load_default_modules,
shell_prompt=shell_prompt,
conn_cls=SshConnection,
is_container=is_container,
max_async=max_async)
Copy link
Collaborator

Choose a reason for hiding this comment

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

might go the extra mile and just remove the link between length of the function name and indentation:

foobarextremelylongandalsowewillchangeittomorrowandbreakindentation(
    iamaparameterandidontcareaboutmyfunctionnamelength=42,
    ...,
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer current indentation scheme which matches Linux kernel style, but set the indentation to 4 spaces here and similar block below (self.android_container = AndroidTarget(...)).

devlib/target.py Outdated
executables_directory=android_executables_directory,
connect=False,
modules=[], # Only use modules with linux target
modules=[], # Only use modules with linux target
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's unrelated to this PR so maybe don't fix it here, but that's a nice antipattern in there: mutable default values. The default value is created when the function is created. Then that specific instance will be used for all function calls where no value is specified:

def foo(x, mylist=[]):
    mylist.append(x)
    return mylist

foo(1)
foo(2)
mylist = foo(3)

assert mylist == [1, 2, 3]

So the moral here is: never use mutable values as defaults. None is not mutable (or it is mutable but has no state to actually mutate, so both views are actually valid), so it makes a nice default. If an empty iterable default was really wanted, tuple() would do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed modules param since it defaults to None already.

Also clean a mutable default value (``modules=[]`` in ``ChromeOsTarget``
class).

Signed-off-by: Metin Kaya <metin.kaya@arm.com>
We can mimic ChromeOS target by combining a QEMU guest (for Linux
bindings of ``ChromeOsTarget`` class) with a Android virtual desktop
(for Android bits of ``ChromeOsTarget``).

Note that Android bindings of ``ChromeOsTarget`` class also requires
existence of ``/opt/google/containers/android`` folder on the Linux
guest.

Signed-off-by: Metin Kaya <metin.kaya@arm.com>
@marcbonnici marcbonnici merged commit b5f311f into ARM-software:master Apr 1, 2024
@metin-arm metin-arm deleted the chromeos branch June 27, 2024 07:45
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.

3 participants