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

Rework sitemap.xml.gz view to fix it for plone.app.multilingual [2.3.x] #92

Merged
merged 3 commits into from
Dec 19, 2016

Conversation

djowett
Copy link
Contributor

@djowett djowett commented May 31, 2016

Tests still required, but this is a suggested fix for #91

@djowett djowett changed the title Rework sitemap.xml.gz view to fix it for plone.app.multilingual Rework sitemap.xml.gz view to fix it for plone.app.multilingual [2.3.x] May 31, 2016
@djowett
Copy link
Contributor Author

djowett commented Jun 1, 2016

@jensens it's a bugfix in my book (for #91), but I wasn't sure which labels should apply to issues and which to PRs. Is there any division? Or can they all apply to both?

@jensens
Copy link
Member

jensens commented Jun 1, 2016

ok, then its a bug (fix) label. I think its obvious that a PR with label bug must be a fix for a bug.

@djowett
Copy link
Contributor Author

djowett commented Jun 1, 2016

@jensens it is - I just wasn't sure if you used these Labels for any special filtering, and didn't want to break things if so ;-)

For example now 01 type: bug makes it look like there are 4 bugs when there are really 3, (this one is a fix for 91 already listed)

Whatever you are happy with is great with me :-)

@jensens
Copy link
Member

jensens commented Jun 1, 2016

As the changelog verifier tells you: Please add a note to CHANGES.rst - the change log file. I just started the tests.

@djowett
Copy link
Contributor Author

djowett commented Jun 1, 2016

@jensens sure - will do. I also need to add tests in plone.app.multilingual & am hoping for someone to add tests in LinguaPlone to make sure we don't break that!

@djowett
Copy link
Contributor Author

djowett commented Jun 1, 2016

@erral I would like to rebase this branch to remove my trial & error from history. Are you (as the only person who has pulled it) OK with that ?

djowett added a commit to plone/plone.app.multilingual that referenced this pull request Jun 1, 2016
@djowett
Copy link
Contributor Author

djowett commented Jun 1, 2016

The tests for PAM 2.x just passed, but we still need tests on PAM 1.x & LinguaPlone

Just had a think through the dependencies, I'd appreciate a few more eyes to review this, as I
haven't taken on a fix with this many dependencies before (in Plone).

@erral @jensens @mauritsvanrees + any others you think should see this.

Plone version multilingual product branch LinguaPlone PAM p.a.layout sitemap Status
Plone 4.3 with LinguaPlone (master) master(*) n/a 2.3.15 working
with PAM 1.x n/a 1.2.3 2.3.15 working
with PAM 2.x n/a 2.0.3 2.3.15 broken
none n/a n/a 2.3.15 working
Plone 5.0 / 5.1 with PAM 3.x / 4.x / master n/a 4.0.1 2.6.1 broken
none n/a n/a 2.6.1 presumed OK

(*) No new release should be required for LinguaPlone unless we break something!

New releases are required for

  • plone.app.layout 2.3.15
  • plone.app.multilingual 1.2.3
  • plone.app.multilingual 2.0.3
  • plone.app.layout 2.6.1
  • plone.app.multilingual 4.0.1

Backports could conceivably be required at some point for PAM 3.x or plone.app.layout 2.5

Version pins
The main issue is not to break the currently working sitemap when plone.app.multilingual 1.x is installed. So 1.2.3 needs to require plone.app.layout 2.3.15
2.0.3 should also require plone.app.layout 2.3.15
The pins for Plone 5 are not so clear to me, is pinning into plone.app.layout 2.6.x branch (rather than 2.5) likely to cause other issues? Maybe @thet you could comment on that?

@erral
Copy link
Member

erral commented Jun 2, 2016

👍 for rebase

@djowett
Copy link
Contributor Author

djowett commented Jun 2, 2016

@erral rebase is done. Can you try cherry-picking the tests from PAM 2.x to PAM 1.x? Can you give any input on the LinguaPlone tests?

@erral
Copy link
Member

erral commented Jun 3, 2016

When cherry-picking the tests, I realized that p.a.layout 2.3.x is intended only for Plone >= 4.3 since version 2.3.11: https://pypi.python.org/pypi/plone.app.layout/2.3.11

Sorry, this comment is a mistake, i was looking at Plone 4.1 test.

@djowett
Copy link
Contributor Author

djowett commented Jun 3, 2016

Ah, great another dependency!

But PAM 1.x works on Plone 4.3 doesn't it?
If so, then tests are still relevant in PAM 1.x

Mikel Larreategi notifications@github.com wrote:

When cherry-picking the tests, I realized that p.a.layout 2.3.x is intended only for Plone >= 4.3 since version 2.3.11:

https://pypi.python.org/pypi/plone.app.layout/2.3.11


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

erral pushed a commit to erral/plone.app.multilingual that referenced this pull request Jun 3, 2016
@erral
Copy link
Member

erral commented Jun 3, 2016

Here is the PAM 1.x pull-request with the changes done in PAM 2.x and the backported tests

plone/plone.app.multilingual#235

@djowett
Copy link
Contributor Author

djowett commented Dec 9, 2016

Update of the above table

Plone version multilingual product (& branch) old sitemap Status p.a.layout fix in Test location Test status
Plone 4.3 none working 2.3.16 p.a.layout (2.3.x) Passes
LinguaPlone (master) working 2.3.16 LinguaPlone Passes
PAM (1.x) working 2.3.16 PAM 1.x Passes
PAM (2.x) broken 2.3.16 PAM 2.x Passes
Plone 5.0 none working 2.5.22 p.a.layout (2.5.x) Passes
PAM (master) broken 2.5.22 PAM master Passes
Plone 5.1 none working 2.6.3 p.a.layout (master) Passes
PAM (master) broken 2.6.3 PAM master Passes
Plone 4.2 none working n/a n/a Not broken, so ain't fixing
LinguaPlone (master) working n/a n/a Not broken, so ain't fixing
PAM (1.x) broken! would be 2.2.x n/a Not fixing at this time

New releases (and tests) are required for

  • plone.app.layout 2.3.16
  • plone.app.layout 2.5.22
  • plone.app.layout 2.6.3

with Pins to the new p.a.layouts in new releases for

  • plone.app.multilingual 2.0.x
  • plone.app.multilingual 5.0.x

and tests (but no releases) that we haven't broken anything in

  • LinguaPlone master (unmerged PR)
  • plone.app.multilingual 1.2.x PR (vs Plone 4.3 & p.a.layout 2.3.16)

A backportmight be required at some point for PAM 3.x

djowett and others added 3 commits December 9, 2016 18:42
…ply such a filter if LinguaPlone is installed.

The filter could (and probably should) be moved to LinguaPlone, but this is an improvement on previous code that assumed
LinguaPlone was the only multilingual addon for Plone
@djowett
Copy link
Contributor Author

djowett commented Dec 18, 2016

Given tests passing in related products. I figure this PR is now ready to merge. Anyone happy to review and do so for me?

cc @jensens @erral @Rudd-O

@jensens
Copy link
Member

jensens commented Dec 19, 2016

lgtm - is there any action needed in master?

@djowett
Copy link
Contributor Author

djowett commented Dec 19, 2016

@jensens Yep there will be, (or should that be in 2.5.x branch?), but I need a merge and release here, so I can pin in p.a.m 2.x to fix the real problem. I'll juggle those balls first, then come back to the others. Ok?

@jensens jensens merged commit facd91f into 2.3.x Dec 19, 2016
@jensens jensens deleted the 2.3.x_issue_91 branch December 19, 2016 11:15
@jensens
Copy link
Member

jensens commented Dec 19, 2016

perfect, thanks a lot!

@djowett
Copy link
Contributor Author

djowett commented Dec 19, 2016

Thanks @jensens who should I ping for a release?

Can I also ask you on whether I need to also fix in 2.5.x branch or master or both? PAM master seems still to be testing against 2.5.x, but could it move to 2.6.x one day?

Is 2.6.x just for Plone 5.1? Changes that you and thet made here mention some "Incompatibilities", but I'm not clear what it IS compatible with :-)
https://github.com/plone/plone.app.layout/blob/master/CHANGES.rst#260-2016-05-10

@jensens
Copy link
Member

jensens commented Dec 19, 2016

@djowett for releases we have the @plone/release-team :)

@jensens
Copy link
Member

jensens commented Dec 19, 2016

IMO its first enough to fix master branch. PAM master tests should run against p.a.layout master branch, since this is all in core now. This should be updated in PAM if needed.

Fixing 1.5.x as well is then more or less just cherry picking.

@djowett
Copy link
Contributor Author

djowett commented Dec 19, 2016

Agreed on all points. Thanks for clarifying

@djowett
Copy link
Contributor Author

djowett commented Dec 19, 2016

@plone/release-team please can we have a release of plone.app.layout from the 2.3.x branch?

@mauritsvanrees
Copy link
Member

Sure. I have released plone.app.layout 2.3.16.

djowett added a commit to plone/plone.app.multilingual that referenced this pull request Dec 19, 2016
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Jan 3, 2017
Branch: refs/heads/master
Date: 2016-12-19T17:35:27Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@16ff77e

Add tests for sitemap.xml.gz (expect to fail for now)

See plone/plone.app.layout#91 and plone/plone.app.layout#92
Also see #209

Files changed:
A src/plone/app/multilingual/tests/test_sitemap.py
M CHANGES.rst
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-19T17:42:35Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@71eb2fa

Use assertIn/assertNotIn  - easier to read (requires Python 2.7)

Files changed:
M src/plone/app/multilingual/tests/test_sitemap.py
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-21T17:50:02Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@7d157d2

Fix tests to use registry rather than portal_properties; unpin plone.app.multilingual so we are testing this source rather than a packaged egg

Files changed:
M src/plone/app/multilingual/tests/test_sitemap.py
M test-plone-5.x.cfg
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T14:29:51Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@58c7556

We shouldn't allow *everything* to fail in Travis!

Files changed:
M .travis.yml
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T14:29:51Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@389aebc

Update failing test - Catalan translations not currently available

Files changed:
M src/plone/app/multilingual/tests/robot/test_translate_content.robot
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T20:15:33Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@62336fc

Require a version of plone.app.layout which contains our sitemap fix

Basically this is:
* 2.6.3+ for Plone 5.1
* 2.5.22+ for Plone 5.0

Files changed:
M setup.py
M test-plone-5.0.x.cfg
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T20:16:55Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@205c047

Try adding plone 5.1 to Travis tests

Files changed:
A test-plone-5.1.x.cfg
M .travis.yml
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2017-01-03T17:47:20+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/plone.app.multilingual@737910b

Merge pull request #255 from plone/master_issue_91-sitemap.xml.gz

Fix sitemap.xml.gz in master (issue #209)

Files changed:
A src/plone/app/multilingual/tests/test_sitemap.py
A test-plone-5.1.x.cfg
M .travis.yml
M CHANGES.rst
M setup.py
M src/plone/app/multilingual/tests/robot/test_translate_content.robot
M test-plone-5.0.x.cfg
M test-plone-5.x.cfg
mister-roboto pushed a commit to plone/buildout.coredev that referenced this pull request Jan 3, 2017
Branch: refs/heads/master
Date: 2016-12-19T17:35:27Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@16ff77e

Add tests for sitemap.xml.gz (expect to fail for now)

See plone/plone.app.layout#91 and plone/plone.app.layout#92
Also see #209

Files changed:
A src/plone/app/multilingual/tests/test_sitemap.py
M CHANGES.rst
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-19T17:42:35Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@71eb2fa

Use assertIn/assertNotIn  - easier to read (requires Python 2.7)

Files changed:
M src/plone/app/multilingual/tests/test_sitemap.py
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-21T17:50:02Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@7d157d2

Fix tests to use registry rather than portal_properties; unpin plone.app.multilingual so we are testing this source rather than a packaged egg

Files changed:
M src/plone/app/multilingual/tests/test_sitemap.py
M test-plone-5.x.cfg
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T14:29:51Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@58c7556

We shouldn't allow *everything* to fail in Travis!

Files changed:
M .travis.yml
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T14:29:51Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@389aebc

Update failing test - Catalan translations not currently available

Files changed:
M src/plone/app/multilingual/tests/robot/test_translate_content.robot
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T20:15:33Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@62336fc

Require a version of plone.app.layout which contains our sitemap fix

Basically this is:
* 2.6.3+ for Plone 5.1
* 2.5.22+ for Plone 5.0

Files changed:
M setup.py
M test-plone-5.0.x.cfg
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2016-12-30T20:16:55Z
Author: Daniel Jowett (djowett) <daniel@jowettenterprises.com>
Commit: plone/plone.app.multilingual@205c047

Try adding plone 5.1 to Travis tests

Files changed:
A test-plone-5.1.x.cfg
M .travis.yml
Repository: plone.app.multilingual
Branch: refs/heads/master
Date: 2017-01-03T17:47:20+01:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/plone.app.multilingual@737910b

Merge pull request #255 from plone/master_issue_91-sitemap.xml.gz

Fix sitemap.xml.gz in master (issue #209)

Files changed:
A src/plone/app/multilingual/tests/test_sitemap.py
A test-plone-5.1.x.cfg
M .travis.yml
M CHANGES.rst
M setup.py
M src/plone/app/multilingual/tests/robot/test_translate_content.robot
M test-plone-5.0.x.cfg
M test-plone-5.x.cfg
talarias pushed a commit that referenced this pull request Dec 9, 2021
fix styles for '#portal-header'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants