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

Discuss components involved in regression #2

Closed
rnveach opened this issue Jun 5, 2017 · 12 comments
Closed

Discuss components involved in regression #2

rnveach opened this issue Jun 5, 2017 · 12 comments
Assignees
Labels

Comments

@rnveach
Copy link
Member

rnveach commented Jun 5, 2017

Discuss and agree on all components project should have at a high level.

By PR, we currently have Git and Generator.

@rnveach
Copy link
Member Author

rnveach commented Jun 5, 2017

I agree with git component. It should scan git and give us what has changed in the PR. It was made into Issue #3 .

Should we have checkstyle component (might be better to name it module to avoid confusion)?
We need to identify the type of file to restrict to only modules (for now). In the future, we also need to identify if the module has new/removed properties as they will need to be included in the regression.

I think we should rename generator to configuration as it will be clearer what we are doing in the package, creating the configuration(s).

We will need report component, which runs the configuration(s) and pushes the reports to a server.

IMO, each component should be completely isolated. We should have an extra package data that will be used to pass data (POJOs) between all the components. Fields in class should be final if possible as 1 component shouldn't need to modify data of another.

@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach
For module component. I agree with the functions you listed. But as pointed at #1 (review). The method was not a good implementation, for it just instantiate the class for the release version of checkstyle, and that is meaningless. I haven't think out a way to load the class from file and meanwhile avoid conflicts by now. Should we discuss it here or create new issue?

I think we should rename generator to configuration as it will be clearer what we are doing in the package

I am OK with it. Are you OK with the class name pattern XXXConfigGenerator like #1 ?

We will need report component, which runs the configuration(s) and pushes the reports to a server.

OK. Should we reuse the existing code in contribution repo?

@rnveach
Copy link
Member Author

rnveach commented Jun 11, 2017

Should we discuss it here or create new issue?

Create new issue, this discussion is for only high level information for us to agree on component names and their basic purpose. Specifics will be done in issue for each component separately.
If we agree on high level details for each component, than create a new issue for all of them except the data package.
Put in all your specifics, questions, issues, and we will discuss things further and start new PRs.

Should we reuse the existing code in contribution repo?

Be more specific, I assume you mean the regression groovy scripts? If so, than yes, we should try to re-use them.
We need a good reason not to be able to use them as they are widely used now.

When discussions here are done and issues are created, we can close this issue.

@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach
I am thinking about the filter. In git component there would be filters depending on path and changed type. But we may need some other filter, i.e. filter to judge whether it is a checkstyle modole. So we might need filters in configuration module as well. What's your option?

@rnveach
Copy link
Member Author

rnveach commented Jun 11, 2017

IMO, filtering based on module should be done in module component. git/configuration shouldn't be trying to dedude checkstyle modules as that puts too much work into the one component and we would probably still end up splitting it based on git/configuration and modules.
configuration should be given only the information it needs to create the configuration(s). This would be the list of modules and the specified properties. So all the filtering should be done by then.

@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach
I agree that git component shouldn't do too many things. So configuration component is only to get data and use them to generate config; module component contains specific strategies. Is that right?

@rnveach
Copy link
Member Author

rnveach commented Jun 11, 2017

Is that right?

Yes, that is correct.

contains specific strategies

I assume you mean based on retrieving default module, module with specific properties, etc.? Then yes.
We should always do all strategies to test all things possible for maximum effect, but for now as we are just starting we can start with the easiest, default module.

@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach I think I have no questions now. We would have git, module, configuration and data.

@rnveach
Copy link
Member Author

rnveach commented Jun 11, 2017

We would have git, module, configuration and data.

And report. :)
Than please start the issues.

This was referenced Jun 11, 2017
@Luolc
Copy link
Contributor

Luolc commented Jun 11, 2017

@rnveach Done. #19 , #20 , #21

@rnveach rnveach closed this as completed Jun 11, 2017
@romani
Copy link
Member

romani commented Jun 25, 2017

We will need report component, which runs the configuration(s) and pushes the reports to a server.

please make publish to and generation of reports as separate steps/modules. Most of the time it will be used without publish as it might become our new standard of testing tool.

@rnveach
Copy link
Member Author

rnveach commented Jun 25, 2017

publish ... reports as separate steps/modules

All modules will be isolated except for their inputs and outputs.
Discussion of arguments to execute different areas will be at #32 . Right now we are focusing on creating the components.

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

No branches or pull requests

3 participants