Skip to content

Update errored version updates version formatting #2233

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

Conversation

HaudinFlorence
Copy link
Contributor

PR Checklist:

  • note any issues closed by this PR with closing keywords
  • if you are adding a new page under docs/ or community/, you have added it to the sidebar in the corresponding _sidebar.json file
  • put any other relevant information below

This PR aims at fixing task 11 of issue #2137
For the errored version updates, it replaces <pre> by <Markdown> components with relevant properties.

@HaudinFlorence HaudinFlorence requested a review from a team as a code owner July 18, 2024 10:47
Copy link

netlify bot commented Jul 18, 2024

Deploy Preview for conda-forge-previews ready!

Name Link
🔨 Latest commit 10f2af3
🔍 Latest deploy log https://app.netlify.com/sites/conda-forge-previews/deploys/6698f2dcba0cbd0008a41128
😎 Deploy Preview https://deploy-preview-2233--conda-forge-previews.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@HaudinFlorence
Copy link
Contributor Author

HaudinFlorence commented Jul 18, 2024

I am not sure whether <a href="https://github.com/...">bot CI job</a> should be parsed in the error message.

@jaimergp
Copy link
Member

jaimergp commented Jul 18, 2024

Probably, yes. But we should fix how the bot emits these errors so it's a better looking Markdown. We could also process the HTML-in-Markdown thingy, but that's a bit too much?

Let me see if I can find how we emit these error blobs.

Edit: Here it is. It boils down to passing the exception message directly, plus some minor massaging. We could maybe prettify the "header", but the exception message is what it is.

The changes in the frontend, as they are, look good to me.

@jaimergp
Copy link
Member

cc @beckermr - we are looking at prettifying the version update errors.

From:

image

To:

image

For best results, we might need to adjust this blob in cf-scripts. Any thoughts?

@beckermr
Copy link
Member

Yes this has been on my todo list and I agree. Let's merge the pr here and we can merge a change in the bot. Unfortunately we may have to go back and edit the existing blobs to make everything look nice.

@jaimergp
Copy link
Member

I see two ways to do this:

  • Make the bot generate markdown. This PR would be sufficient.
  • Make the bot generate JSON payloads. We would need to parse the JSON a bit here before merging. I imagine a schema like:
{
  "status": str,  // I guess we only have 'error' here?
  "n_attempts": int,
  "job_url": str,
  "branch": str,
  "message": str
}

I think long term JSON makes more sense but I don't know how tricky that would be. WDYT?

@beckermr
Copy link
Member

What do we do for the migration errors? We should follow that maybe?

@beckermr
Copy link
Member

@jaimergp for the version updates, we need to adjust this line: https://github.com/regro/cf-scripts/blob/98f3ea00203e7a7d33d43e1d6c791e2227d778f1/conda_forge_tick/auto_tick.py#L632 The one you linked is for plain migrators.

@beckermr
Copy link
Member

Oh weird, now I see both things in both places.

@beckermr
Copy link
Member

OK see this PR: #2234

@jaimergp
Copy link
Member

Thanks @HaudinFlorence. Closing this one in favor of #2234. We might revisit this section of the status page in the future, though. I'll keep you posted!

@jaimergp jaimergp closed this Jul 19, 2024
@HaudinFlorence
Copy link
Contributor Author

Thanks @HaudinFlorence. Closing this one in favor of #2234. We might revisit this section of the status page in the future, though. I'll keep you posted!

@jaimergp Ok. Thanks for the input.

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