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

Why does plone/documentation keep getting committed automatically? #768

Closed
stevepiercy opened this issue Jan 24, 2022 · 23 comments · Fixed by plone/mr.roboto#91
Closed

Why does plone/documentation keep getting committed automatically? #768

stevepiercy opened this issue Jan 24, 2022 · 23 comments · Fixed by plone/mr.roboto#91

Comments

@stevepiercy
Copy link
Contributor

I keep breaking Jenkins with ever merge commit to the 6-dev branch in plone/documentation. Help! Make it stop!

This file keeps getting updated:

https://github.com/plone/buildout.coredev/blob/6.0/checkouts.cfg

Example automatic commit:

75b42d6

@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

As mentioned elsewhere this can be mitigated by adding:

documentation                          = git ${remotes:plone}/documentation.git pushurl=${remotes:plone_push}/documentation.git egg=false branch=6-dev path=${buildout:docs-directory}

But probably removing this hook might help.

@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

I will try to fix the sources right now and investigate more on this later CC @plone/testing-team

@gforcada
Copy link
Sponsor Contributor

🤔 I think is a mr.roboto feature, if a package that is listed in sources gets a new change but is not on checkouts, it adds it, so one can ensure that the new features that landed on a package get tested

@gforcada
Copy link
Sponsor Contributor

We can add an exception list on that functionality (if it actually works as I mentioned, which my not even be true 😅 )

@stevepiercy
Copy link
Contributor Author

I do not know why it was added or whether it should be preserved in some way.

I now have access to the plone/documentation repo settings, too.

ale-rt added a commit that referenced this issue Jan 24, 2022
@stevepiercy
Copy link
Contributor Author

Also this is not urgent from my perspective or from that of the branch 6-dev in Plone Docs. Take your time. I just don't want to keep breaking Jenkins builds!

@gforcada
Copy link
Sponsor Contributor

@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

@gforcada I think you got it right but in the sources.cfg the repo is defined as:

docs                          = git ${remotes:plone}/documentation.git pushurl=${remotes:plone_push}/documentation.git egg=false branch=6-dev path=${buildout:docs-directory}

In #769 I provide an alias for that.
Maybe we should just switch docs to documentation, but I am afraid of implications I cannot see right now

@gforcada
Copy link
Sponsor Contributor

the question is, why is documentation on the checkouts? and if anyway it has to stay there, it does at least not have to make jenkins run again, right? 🤔 so we can add an exception list on the function on mr.roboto or even remove the hooks for mr.roboto on the documentation repo itself... either of the two will fix this problem

@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

I think that somewhere here we could make some check that excludes the documentation repo: https://github.com/plone/mr.roboto/blob/70e08fcab664a8db4e0749b92c6acca62288d46d/src/mr.roboto/src/mr/roboto/subscriber.py#L459-L469

@gforcada
Copy link
Sponsor Contributor

sure, why is it though that the documentation is checked alongside the code? 🤔

@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

I have time to do that now, if somebody wants to join me on that, please ping me in discord

@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

sure, why is it though that the documentation is checked alongside the code? thinking

Maybe it is needed for some other automation, IMO it is not that bad to have that in the sources.cfg.

@gforcada
Copy link
Sponsor Contributor

@ale-rt if you can review this: plone/mr.roboto#91 👍🏾

ale-rt added a commit that referenced this issue Jan 24, 2022
@ale-rt
Copy link
Member

ale-rt commented Jan 24, 2022

@gforcada, PR approved, in the meanwhile I removed the unwanted checkout to restore the build.

Also, #769 seems to be a workaround, but I will close it since plone/mr.roboto#91 fixes the root cause of the issue.

@gforcada
Copy link
Sponsor Contributor

Deployed to production, can someone test if my fix does work? 😄

@stevepiercy
Copy link
Contributor Author

We have a few WIP PRs. I'll let you know whether I get another alert after we merge the next one.

@stevepiercy
Copy link
Contributor Author

stevepiercy commented Feb 3, 2022

@gforcada PR #1160 merged. 🤞

@stevepiercy
Copy link
Contributor Author

I think it worked. This file was not updated. https://github.com/plone/buildout.coredev/blob/6.0/checkouts.cfg Is that what y'all were expecting?

@stevepiercy
Copy link
Contributor Author

Not sure what to make of this email. Is it still a problem?

POSSIBLE CHECKOUT ERROR plone/documentation 6-dev

Repository: plone/documentation
Author: stevepiercy <web@stevepiercy.com>
Plone Version: 6.0

This package is used by Plone coredev 6.0 branch,
but it’s not currently checked out so is not including
your change for testing.

If you were merging a pull request,
worry not mr.roboto took care of it already,
still double check checkouts.cfg:

https://github.com/plone/buildout.coredev/blob/6.0/checkouts.cfg

On the other hand, if you made a direct commit to a branch,
add the package to checkouts.cfg (above link) please.

Regards,
mr.roboto

@gforcada
Copy link
Sponsor Contributor

gforcada commented Feb 3, 2022

That's a safe check to ensure that if something goes wrong you get a heads up about it

@stevepiercy
Copy link
Contributor Author

Sorry for being ignorant. I'm new to this repo.

I don't know what is "expected". How do I know what is "something goes wrong"?

@gforcada
Copy link
Sponsor Contributor

gforcada commented Feb 3, 2022

i.e. regular Plone packages are expected to be in a certain buildout.coredev branch, if a package is not check out it might miss the new changes being tested

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 a pull request may close this issue.

3 participants