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

Let's eat our own dogfood #577

Closed
krzyk opened this issue Jan 1, 2016 · 33 comments
Closed

Let's eat our own dogfood #577

krzyk opened this issue Jan 1, 2016 · 33 comments

Comments

@krzyk
Copy link
Collaborator

krzyk commented Jan 1, 2016

Before merging each code we should run newly built qulice against qulice codebase.
This will allow us to catch errors in our checks earlier.

We should have another travis build (only travis) that will do the validations using newly built plugin - it is because sometimes we introduce incompatible changes that could fail on our current codebase, but we can't updated it because it will fail on the previous qulice plugin.
So we should have a build that will be just an information for us, what new violations are being detected. This should allow us to fix invalid ones.

Options how to do it:
1.

  • mvn clean install
  • mvn com.qulice:qulice-maven-plugin:1.0-SNAPSHOT:check -Pqulice (here we will probably need to add excludes e.g. -Dqulice.excludes="xml:.*,checkstyle:.*,pmd:.*/src/test/resources/.*")
  • add option to define qulice version from command line (by using <properties> tag)
  • mvn clean install
  • mvn versions:set "-DnewVersion=1.1"
  • mvn clean install -Pqulice -Dqulice.version=1.0-SNAPSHOT
  1. Any other ideas

For the 1 and 2, we should also make sure that we run the build for all the modules, so some shell magic will be needed to enter each directory and build given plugin.

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 1, 2016

@davvd valid bug

@mkordas
Copy link
Contributor

mkordas commented Jan 1, 2016

@krzyk how would you configure exclusions for such run?

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 1, 2016

@mkordas if they are required (shouldn't, if we add new check we should fix problems that it causes in qulice using e.g. puzzles) it reads excludes from the pom.xml files (and from the code).

@davvd
Copy link

davvd commented Jan 4, 2016

@davvd valid bug

@krzyk tagged this issue with "bug"

@davvd
Copy link

davvd commented Jan 6, 2016

@krzyk thanks for the ticket, your account was topped for 30 mins, payment 73942602

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 13, 2016

@davvd this is postponed

@davvd
Copy link

davvd commented Jan 14, 2016

@davvd this is postponed

@krzyk got it, "postponed" label here

@davvd
Copy link

davvd commented Jan 14, 2016

@davvd this is postponed

@krzyk I will ask somebody else to pick this up

@yegor256 yegor256 mentioned this issue Jan 14, 2016
@krzyk
Copy link
Collaborator Author

krzyk commented Jan 21, 2016

@mkordas I've updated the description, would you like to work on this?

@mkordas
Copy link
Contributor

mkordas commented Jan 21, 2016

@krzyk first we need to update Qulice to 0.15.4 for our code, there are many violations. Should I raise issue for that?

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 21, 2016

@mkordas I don't think we need to upgrade for this issue specifically, this will be just a build informing us of the violations.
But yes, there are quite a lot of changes needed for the new qulice, so an issue will be needed.

@mkordas
Copy link
Contributor

mkordas commented Jan 21, 2016

@krzyk OK, I'll submit an issue with some proposals how to fix missing since tag. So you propose to allow failing Travis build for some time and red mark on each commit?

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 21, 2016

@mkordas we won't be able to always have a green build, e.g. when we add a code that reverses the previous check
A red flag is OK, if I can go and see that the only failing one is the "dogfooded" qulice.

In the future if the newly added rule is not conflicting with previous qulice, we will fix the code during the implementation of the check (or add a todo if it is more work).

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 24, 2016

@mkordas are you interested?

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 26, 2016

@mkordas ping

@mkordas
Copy link
Contributor

mkordas commented Jan 26, 2016

@krzyk because of @davvd being so slow I'll ask for an assignment only when my PR is ready and has assignee. I don't want to wait 4 days just for code review.

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 26, 2016

@mkordas yes, he is a pain, I'm just asking if I should look for someone else :)

@mkordas
Copy link
Contributor

mkordas commented Jan 26, 2016

@krzyk I'll give a try :)

mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
For yegor256#577:

* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
For yegor256#577:

* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
For yegor256#577:

* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
For yegor256#577:

* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
For yegor256#577:

* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
For yegor256#577:

* added new build to execute freshly built Qulice over entire code
* due to Travis YML syntax limitations two different commands must be
 specified by environmental variables
mkordas added a commit to mkordas/qulice that referenced this issue Jan 26, 2016
@mkordas
Copy link
Contributor

mkordas commented Jan 26, 2016

@krzyk please assign me here

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 27, 2016

@davvd assign @mkordas please

@davvd
Copy link

davvd commented Jan 27, 2016

@davvd assign @mkordas please

@krzyk OK @mkordas please proceed, this task is yours

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 27, 2016

@davvd this is not postponed

@mkordas
Copy link
Contributor

mkordas commented Jan 27, 2016

@davvd please see PR #650

@davvd davvd removed the postponed label Jan 27, 2016
@davvd
Copy link

davvd commented Jan 27, 2016

@davvd this is not postponed

@krzyk sure, thanks, I removed "postponed" tag to it

@davvd
Copy link

davvd commented Jan 27, 2016

@davvd please see PR #650

@mkordas will check it soon, thanks

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 27, 2016

@davvd assign @mkordas please

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 27, 2016

@mkordas does the task appear in your agenda? I don't see you assigned in github, but David confirmed it.

@mkordas
Copy link
Contributor

mkordas commented Jan 27, 2016

@krzyk yes, I see it. Can you close the issue?

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 27, 2016

@mkordas thanks

@krzyk krzyk closed this as completed Jan 27, 2016
@davvd
Copy link

davvd commented Jan 28, 2016

@davvd assign @mkordas please

@krzyk yes, @mkordas is already assigned to this task, please go ahead!

@krzyk
Copy link
Collaborator Author

krzyk commented Jan 28, 2016

@davvd this is not postponed

@davvd
Copy link

davvd commented Jan 28, 2016

@davvd this is not postponed

@krzyk got it, "postponed" tag removed from here

@davvd davvd removed the postponed label Jan 28, 2016
@davvd
Copy link

davvd commented Jan 29, 2016

@mkordas 1 hour sent to your balance (ID 56ab34d415431a575200001a), many thanks! It took zero.. there is a bonus for fast delivery (m=0). +60 added to your rating, current score is: +4108

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

No branches or pull requests

3 participants