Skip to content

Proposal for "Refactoring of Common Modules" #1

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

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

Conversation

dboltovskyi
Copy link
Owner

@dboltovskyi dboltovskyi commented Nov 7, 2017

Proposal for Refactoring of Common Modules

Common modules consist of commonly used functions

Current downsides:
● No single standard
● Duplicates
● Lack of description
● A lot of outdated or unused code

To Do:
● Design a standard template for common functions
● Split all functions into two groups: Utils and Common sequences
● Split these two huge groups between a few modules (based on functionality)
● Update all existing test scripts

@dboltovskyi dboltovskyi changed the title Test scripts common modules refactoring Proposal for "Refactoring of Common Modules" Nov 7, 2017
@dev-gh
Copy link

dev-gh commented Nov 7, 2017

@dboltovskyi
What is "Common modules" ? Add some meaningful description to PR


## Introduction

ATF Test Scripts use a lot of commonly used functions. Such as:
Copy link

Choose a reason for hiding this comment

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

@dboltovskyi
Would be good to put link here to repository


## Proposed solution

### Leave existing modules as is
Copy link

Choose a reason for hiding this comment

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

@dboltovskyi
Why to mention that if proposal is about what to change basically?


## Potential downsides

Existing ATF test scripts may need to be updated if some function from new common modules is required.
Copy link

Choose a reason for hiding this comment

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

@dboltovskyi
I think its better to say "must be updated when new common modules will be used in the test script". Otherwise to me it sounds like "well, you may still continue to use old code as long as you want, don't bother to update anything"


## Introduction

ATF Test Scripts use a lot of commonly used functions. Such as:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi ,

suggest to rewrite Test Scripts to test scripts
also, it would be better like
ATF test scripts use a lot of functions like:

- perform policy table update
- etc.

The purpose of this proposal is to clean up these common modules.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi ,

suggest to add sentence before
All these functionality are not tests artifacts but tools that tests use.

- perform policy table update
- etc.

The purpose of this proposal is to clean up these common modules.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi ,
and "The purpose of this proposal is to clean up these common modules." better to move to solution


## Motivation

Current downsides of common modules are:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi ,

Please, move this to downside section of template

Copy link
Owner Author

Choose a reason for hiding this comment

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

@Itileda Does 'Potential downsides' section mean? If so, 'Potential downsides' section is about downsides of new implementation.
But current comment is related to downsides of existing implementation


## Proposed solution

### Leave existing modules as is
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi
please add (backward compatibility)

- utils - folder for modules with utility functions (e.g.: data accessors, table functions, data converters, etc.)
- sequences - folder for modules with common sequences (e.g.: register/activate mobile application, policy update, put file, etc.)

TBD: Names of the module files.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi ,
please remove TBD.


## Introduction

ATF Test Scripts use a lot of commonly used functions. Such as:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi better to say "a lot of functions in commonly used modules"

By leaving all existing modules currently located in ./user_modules folder as is
it will be possible to use all existing ATF test scripts without changes.

### Design a standard template for functions
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi standart

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi You are right. Correct word is STANDARD

- utils - folder for modules with utility functions (e.g.: data accessors, table functions, data converters, etc.)
- sequences - folder for modules with common sequences (e.g.: register/activate mobile application, policy update, put file, etc.)

TBD: Names of the module files.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi what is TBD?

### Follow new approach when developing new ATF test scripts

- In newly created ATF scripts use only functions from new "common_modules" folder and not from an old "user_modules" one
- If some utility or common sequence function doesn't exist create it in a new "common_modules" in appropriate module file using standard template
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi standart

By leaving all existing modules currently located in ./user_modules folder as is
it will be possible to use all existing ATF test scripts without changes.

### Design a standard template for functions
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi You are right. Correct word is STANDARD


### Transfer most used common functions into a new folder

Most used functions needs to be transferred from existing "user_modules" to a new "common_modules" folder in appropriate module file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi needs-> need as functions are plural

### Follow new approach when developing new ATF test scripts

- In newly created ATF scripts use only functions from new "common_modules" folder and not from an old "user_modules" one
- If some utility or common sequence function doesn't exist create it in a new "common_modules" in appropriate module file using standard template
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi Use at least - between exist and create

### Transfer most used common functions into a new folder

Most used functions needs to be transferred from existing "user_modules" to a new "common_modules" folder in appropriate module file.
And they also need to be updated according to a new template.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dboltovskyi I propose to avoid to start sentence from And.
I think Aslo they need to be updated... sounds better.

@dboltovskyi
Copy link
Owner Author

@Itileda , @dev-gh , @AByzhynar , @AKalinich-Luxoft please check new commit e932de8

@dev-gh
Copy link

dev-gh commented Nov 8, 2017

@dboltovskyi
By asking "What is "Common modules" ?" I basically wanted you to mention ATF. Otherwise it sounds like "common modules of SomeFamousApplicationEverybodyShouldBeAwareOf"


The purpose of this proposal is to clean up these common modules.

### Leave existing modules as is for backward compatibility
Copy link

Choose a reason for hiding this comment

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

@dboltovskyi
If you insist on keeping this point in the list (however I don't see why it is mentioned as proposal is about something to change) I recommend to put it at the end and don't mark it as point for changes.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@dev-gh I would like to keep this point

Copy link
Owner Author

Choose a reason for hiding this comment

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

@dev-gh Regarding the comment "What is "Common modules"
Common modules is not part of ATF - it's part of test scripts

dboltovskyi pushed a commit that referenced this pull request Mar 11, 2020
* SendLocation for Mobile Nav

This proposal is about extending the capability for apps to receive and service SendLocation requests from other apps which is currently limited to the Head Unit's embedded navigation system.

* Updating links to assets

* Updated links to assets

* Create a

* Asset #1

* asset #2

* Delete a

* Delete SendLocationForMobileNav_RAI.jpg

* Add files via upload

* Delete SendLocationForMobileNav_RequestResponse.jpg

* Add files via upload

* Addressed comments
dboltovskyi pushed a commit that referenced this pull request Mar 11, 2020
…on) (smartdevicelink#644)

* Add revision fixes

Minor changes #1

Update 0205-Avoid_custom_button_subscription_when_HMI_does_not_support.md

Updates after review#2

* Update 0205-Avoid_custom_button_subscription_when_HMI_does_not_support.md
dboltovskyi pushed a commit that referenced this pull request Mar 11, 2020
* Revision fixes

fixing typos

Updates after review #1

Update 0206-remote_atf_testing.md

Minor updates

Minor updates #2

minor text formatting

* Fixes after Laura review

* Update 0206-remote_atf_testing.md

* Update 0206-remote_atf_testing.md

* Update 0206-remote_atf_testing.md
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.

7 participants