We should be careful that
stable will not run behind
main more than necessary.
It seems like we never synced
main when Discourse 3.0 was released (which is a typical moment to sync them, since Discourse core is also synced then (Discourse 3.0 is 100% equal to Discourse 3.1.0 beta 1).
We should test whether the current main branch passes all tests on Discourse 3.0.
If so, we should make
stable point to the current commit on
main and keep it synchronized until we have an actual new feature that is not compatible with Discourse 3.0.
If not, we should see if there are any large issues and fix them if the effort is relatively low
Discourse 3.1 has been released so this is the perfect moment to perform this
we should make
stable point to the current commit on
main and keep it synchronized until we have an actual new feature that is not compatible with Discourse 3.2.0 beta-something.
This is also interesting.
Assigning to the maintainer since this is important but not very urgent (although it would be nice to do this somewhere in the upcoming week and treat it as top-of-the-stack). Maintainer can then decide who will perform the task.
@jumagura Could you have a look at this one? We can discuss it in our next meeting.
When is the next meeting? This should be only 5 minutes of work?
Hi Richard, Angus
I’ve created a PR for merging main into stable (link). While all tests passed locally on Discourse stable, a frontend test is failing in the GitHub CI environment.
The error is related to a modal change I made 5 months ago:
Error: QUnit Test Failure: Global error: Uncaught Error: Assertion Failed: Could not find "modal.insert-hyperlink" template, view, or component. at http://localhost:7357/assets/vendor.js, line 43396
I’m unable to identify the cause, as the assertion in the error doesn’t match what I’ve done.
Here are the relevant changes:
- Addition of the modal: link
- Acceptance test: link
Could you please help me understand what might be going wrong? My apologies if I’m overlooking something simple.
I think the test is failing because the code needs updating (so the test is working, i.e. it’s showing us there’s a problem). The issue is probably also present in main. It looks like we’re still using the old modal api in the custom wizard.
discourse/discourse now uses
Let me know if you run into issues when addressing this in main and stable.
Yeah, this was a change recently.
On TLP I had to change a modal like so, you may need to do something similar:
Note this required a compatibility pin but if you are running two branches check if the change is also necessary on stable.
Any idea why PMS doesn’t find that error then?
I’ll let Marcos have a shot at answering that. I don’t know if I’m right yet.
I’m reaching out to provide an update on the modal component issue I’ve been working on. Here’s a detailed breakdown of the situation:
Attempt to Use the Template: My initial approach involved utilizing the existing Discourse modal template
insert-hyperlink. However, this is where I encountered issues with the CLI tests, even though the functionality appears to be working.
Suspecting the Need to Create a Component: I suspect that I may need to create a dedicated component instead of relying on the existing Discourse template. The new modal system in Discourse seems to require components, and the template alone may not be sufficient.
Funny Twist: In an interesting turn of events, I had previously deleted a component that performed that function (apologies to @Angus) and replaced it with the template. It seems that I’ll need to restore that component or create a new one to align with Discourse’s updated modal system.
It’s a bit of a learning curve, and I’m still exploring the best approach. If anyone has faced similar challenges or has insights into working with the new modal system in Discourse, I’d appreciate any guidance or feedback.
Thanks for your continued support, and I look forward to any insights you may have!
It sounds like you’re on the right track Marcos. Keep going. You’ll get there.
I was missing a
, that was the problem. I should have read Robert code better I fixed that deprecation problem, now all test are passing.
I created a PR to main and from there it would be merged to stable.
Reviewed, ran tests locally, manually tested as well and merged with thanks!
Does this need to be on stable too?
btw, one observation: the admin FE tests run quite long - do we know why that is the case? There seem to be a lot of long pauses?
There are still a lot of differences between
main, also when I do
diff -r between them.
It might be best to remove the stable branch and create a new, pristine, one.
I can clarify that situation. When data is created, modified, or deleted in the admin tab(custom fields and manager tabs), a banner displays a message that returns to normal after 10 seconds. The test asserts this behavior in those six actions.
I initially tried to assert only the first change or even ignore it, but this led to a global failure. The error occurred after this test passed and other tests started running. It was caused when the message tried to return to its default state, and no object was found since another test was running.
I resolved the issue by asserting both the change and the return to normal, ensuring that the test functions correctly without errors.
We could do that. I agree with that. That would reduce to zero any merge problems
is there anyway we can shorten that 10 seconds do you think?
Following @richard 's idea to synchronize the
stable branch with
main, here are the steps we can take:
- Reset the
stable branch to the
- Force-push the
stable branch to the remote repository.
Optionally, we can create a backup of the current
stable branch to preserve its history.
This approach will align
main without losing the history of changes.
@angus, do you agree with this approach? If so, we can proceed with these steps. If you have any concerns or alternative suggestions, please let me know.
I understand the need for efficiency. I’ll look into options to reduce this time. Will keep you updated.
What about a very simple solution ?
Rename current stable to
Then branch current state of
main to a new
@richard would that work for you?
That works - but only once so let’s call it stable_30 then.