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

editor/code: Update dependencies #15265

Merged
merged 7 commits into from
Jul 13, 2023

Conversation

tetsuharuohzeki
Copy link
Contributor

@tetsuharuohzeki tetsuharuohzeki commented Jul 11, 2023

This includes:

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2023
@davidbarsky
Copy link
Contributor

Update @types/vscode to 1.79

Sorry, slightly odd question that I don't think I've asked you before: do you mind keeping rust-analyzer at 1.78? For work-related reasons, a bunch of folks at my employer are on 1.78 and will be on it for a while longer, and it'll be a second before they're on something higher. It's not trivial to upgrade for reasons that I'm not entirely sure I'm allowed to say.

(I feel bad asking this question, but given that I'm not aware of any meaningful extension-related changes that occurred between 1.78–1.80, I don't feel too bad.)

@bors
Copy link
Contributor

bors commented Jul 12, 2023

☔ The latest upstream changes (presumably #15264) made this pull request unmergeable. Please resolve the merge conflicts.

@tetsuharuohzeki
Copy link
Contributor Author

Sorry, slightly odd question that I don't think I've asked you before: do you mind keeping rust-analyzer at 1.78? For work-related reasons, a bunch of folks at my employer are on 1.78 and will be on it for a while longer, and it'll be a second before they're on something higher. It's not trivial to upgrade for reasons that I'm not entirely sure I'm allowed to say.

Ah, Okay, I removed to update about it.

@tetsuharuohzeki
Copy link
Contributor Author

rebased on the lastest master.

@lnicola
Copy link
Member

lnicola commented Jul 13, 2023

How did you format the code, and can we run it automatically (e.g. to check it on CI)?

@tetsuharuohzeki
Copy link
Contributor Author

@lnicola

How did you format the code, and can we run it automatically (e.g. to check it on CI)?

I did it by running npm run fix in /editor/code/.

And our CI already have the step checking the formatting result by here:

- run: npm run lint
working-directory: ./editors/code
if: needs.changes.outputs.typescript == 'true'


IMO, it might be nice to split these ci steps into separated jobs to make clarify what we checking.

@lnicola
Copy link
Member

lnicola commented Jul 13, 2023

Oh, right. I think it's good enough, you can see what failed. Maybe we could add a comment along the lines of "try npm run fix if this fails", for people less used to Node and npm.

@tetsuharuohzeki
Copy link
Contributor Author

I'll open it as an another pull request

@lnicola
Copy link
Member

lnicola commented Jul 13, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jul 13, 2023

📌 Commit 085b755 has been approved by lnicola

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 13, 2023

⌛ Testing commit 085b755 with merge c7ce8ad...

@bors
Copy link
Contributor

bors commented Jul 13, 2023

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing c7ce8ad to master...

@bors bors merged commit c7ce8ad into rust-lang:master Jul 13, 2023
@tetsuharuohzeki tetsuharuohzeki deleted the update-dependencies branch July 14, 2023 17:22
bors added a commit that referenced this pull request Jul 22, 2023
editor/code: Break down CI steps to know what is failing easily

This do the thing I mentioned in #15265 (comment)

This aims to improve CI status check more readable.

I tried to use [`jobs.<job_id>.if`](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idif) to make the configuration
more shortly once.

But it could not fire the `end-success` or `end-failure` status if some jobs in the workflow were skipped. This causes an integration problem with bors.

By their reasons, this patch still uses `jobs.<job_id>.steps[*].if`.

---

To do this change, we reorganize npm-script.

| previous           | after                                  |
|--------------------|----------------------------------------|
| `npm run lint`     | `npm run lint && npm run format:check` |
| `npm run fix`      | `npm run lint:fix && npm run format`   |

The previous `npm run fix` sometimes does not complete fix automatically because ESLint's autofix doees not follow prettier's formatting. So we need to run `npm run lint:fix && npm run format` by this order.
bors added a commit that referenced this pull request Jul 24, 2023
…75, r=lnicola

vscode: change minimum VS Code version to 1.75 from 1.78

I previously mentioned [in a comment](#15265 (comment)) that folks at my employer are on 1.78, but I was incorrect: people are on 1.75. I know this is outside the standard version support policy, but I haven't seen any of the new features in VS Code be used in rust-analyzer. The most applicable feature in those three versions of VS Code that I can find [is lazily resolving code actions](https://code.visualstudio.com/updates/v1_78#_resolve-code-action-commands-in-resolvecodeaction), but it doesn't seem like rust-analyzer is making use of that feature.

(I'll be posting a PR that adds support for `$saved_file` within the next day, so feel free to bump the minimum VS Code version back to 1.78 if/when that PR lands. This just makes vendoring the _actual_ change I'm interested in a little bit easier.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants