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

[Extensions] Incompatible modules get saved now #2220

Merged
merged 2 commits into from
Feb 2, 2015
Merged

[Extensions] Incompatible modules get saved now #2220

merged 2 commits into from
Feb 2, 2015

Conversation

Portugao
Copy link

@Portugao Portugao commented Feb 2, 2015

Incompatible modules get saved into the database now with state set to
incompatible.

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1641
Refs tickets -
License MIT
Doc PR -
Changelog updated yes

Incompatible modules get saved into the database now with state set to
incompatible.

| Q                 | A
| ----------------- | ---
| Bug fix?          | yes
| New feature?      | no
| BC breaks?        | no
| Deprecations?     | no
| Tests pass?       | yes
| Fixed tickets     | #1641
| Refs tickets      | -
| License           | MIT
| Doc PR            | -
| Changelog updated | yes
@Guite
Copy link
Member

Guite commented Feb 2, 2015

Thanks for the patch!
Could you please create another PR for the 1.4 branch?
@craigh @cmfcmf please correct me if this is wrong.

@Guite Guite added this to the 1.3.10 milestone Feb 2, 2015
@Portugao
Copy link
Author

Portugao commented Feb 2, 2015

Puh. I was happy to be able to create this PR.
How to do for 1.4?

@craigh
Copy link
Member

craigh commented Feb 2, 2015

yes, we need PR's for both branches at the same time so they do not get out of sync.

@@ -1434,7 +1453,7 @@ public function updatehooks($args)

// Delete hook regardless
$where = "WHERE $hookscolumn[smodule] = '" . DataUtil::formatForStore($modinfo['name']) . "'
AND $hookscolumn[tmodule] <> ''";
AND $hookscolumn[tmodule] <> ''";
Copy link
Member

Choose a reason for hiding this comment

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

I disagree with this (and following) formatting change

@craigh
Copy link
Member

craigh commented Feb 2, 2015

most of this looks OK I think. you can make changes locally and push a new commit to the same branch and the changes will be reflected in the PR (if you did not already know this).

@craigh
Copy link
Member

craigh commented Feb 2, 2015

yes, we need PR's for both branches at the same time so they do not get out of sync.

@Portugao - check in 1.4. and see if the problem even exists there. I worked on this issue I think a while ago and maybe already fixed it there. please check.

@craigh
Copy link
Member

craigh commented Feb 2, 2015

for example in 1.4 I have this:

screen shot 2015-02-02 at 8 17 20 am

@craigh
Copy link
Member

craigh commented Feb 2, 2015

Is that what you would expect?

@Portugao
Copy link
Author

Portugao commented Feb 2, 2015

@craigh Yes.

@craigh
Copy link
Member

craigh commented Feb 2, 2015

👍 great! so no need for a 1.4 PR 😉

@Portugao
Copy link
Author

Portugao commented Feb 2, 2015

I checked too, it works for me in 1.4. Fine.

craigh added a commit that referenced this pull request Feb 2, 2015
[Extensions] Incompatible modules get saved now
@craigh craigh merged commit 43eab9f into zikula:1.3 Feb 2, 2015
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