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

Remove outdated and unused Impact Analysis / Requirements code #219

Merged
merged 3 commits into from
Nov 13, 2016

Conversation

balsdorf
Copy link
Contributor

@balsdorf balsdorf commented Nov 7, 2016

This code is not being used and possibly never has been so I'm removing it.

Before upgrading, users should backup their database or check that these tables / fields are empty:

  • issue_requirement (table)
  • issue.iss_impact_analysis (field)

@balsdorf
Copy link
Contributor Author

balsdorf commented Nov 7, 2016

@glensc Do you think we should remove all trace of this feature from the database? I see:

  • issue_requirement table.
  • history_type items
  • issue.iss_impact_analysis field

There should not be any data in any of these tables / fields but I worry about silently dropping data. Thoughts?

@glensc
Copy link
Member

glensc commented Nov 9, 2016

We could leave free form text in changelog for the next release (which is usually copied to release notes when release is tagged). And upgrade instructions say anyway that you should backup database and files prior upgrade.

So I'm up for cleaning up stale data.

@glensc glensc added this to the 3.1.5 milestone Nov 9, 2016
Added free form warning to Changelog
@balsdorf
Copy link
Contributor Author

@glensc I've added the patch to drop the legacy tables / fields and added the warning to the Changelog. This is ready to merge if everything looks good to you.

@glensc glensc merged commit 10f11eb into master Nov 13, 2016
@glensc glensc deleted the impact_analysis branch November 13, 2016 10:01
glensc added a commit that referenced this pull request Nov 13, 2016
@glensc
Copy link
Member

glensc commented Nov 23, 2016

updated to 3.1.5 and get these in errors:

2016-11-23 23:44:34: (mod_fastcgi.c.2694) FastCGI-stderr: PHP message: PHP Notice:  Undefined index: iss_impact_analysis in /usr/share/eventum/lib/eventum/class.issue.php on line 2708
2016-11-23 23:44:34: (mod_fastcgi.c.2694) FastCGI-stderr: PHP message: PHP Fatal error:  Class 'Impact_Analysis' not found in /usr/share/eventum/src/Controller/ViewController.php on line 229

meh?

@glensc
Copy link
Member

glensc commented Nov 23, 2016

come on @balsdorf you did not finish your job! you did not even do code search for removed classes and fields?! did you even test it?

@glensc
Copy link
Member

glensc commented Nov 23, 2016

$ grep -r Impact_Analysis .
./src/Controller/PopupController.php:21:use Impact_Analysis;
./src/Controller/PopupController.php:117:                $res = Impact_Analysis::insert($this->issue_id);
./src/Controller/PopupController.php:122:                $res = Impact_Analysis::update($this->isr_id);
./src/Controller/PopupController.php:127:                $res = Impact_Analysis::remove();
./src/Controller/ViewController.php:29:use Impact_Analysis;
./src/Controller/ViewController.php:229:                'impacts' => Impact_Analysis::getListing($this->issue_id),

$ grep -r iss_impact .
./lib/eventum/class.issue.php:2708:        $res['iss_impact_analysis'] = nl2br(htmlspecialchars($res['iss_impact_analysis']));
./lib/eventum/class.issue.php:2962:                    iss_impact_analysis=?
./upgrade/patches/59_remove_impact_analysis.sql:2:ALTER TABLE {{%issue}} DROP COLUMN iss_impact_analysis;
./upgrade/schema.sql:159:  iss_impact_analysis text,

$ 

@balsdorf
Copy link
Contributor Author

Ah shit, my ide was only searching in templates. Damnit. Totally failed on this.

@glensc What is correct git procedure since this has already been merged? Create a new PR that references this?

@glensc
Copy link
Member

glensc commented Nov 28, 2016

i would just commit and include ticket id at the end of commit first line: #219

but if you have more than one commit or want to review the changes, create new branch, create PR, and in PR body refer to this ticket.

balsdorf pushed a commit that referenced this pull request Nov 28, 2016
balsdorf pushed a commit to mariadb-corporation/eventum that referenced this pull request Dec 14, 2016
balsdorf pushed a commit to mariadb-corporation/eventum that referenced this pull request Dec 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants