Skip to content

Remove broken format script#991

Merged
floehopper merged 1 commit intomainfrom
remove-broken-format-script
Apr 29, 2024
Merged

Remove broken format script#991
floehopper merged 1 commit intomainfrom
remove-broken-format-script

Conversation

@floehopper
Copy link
Copy Markdown
Contributor

This script is broken, because no .prettierrc file is included in the repo.

We're relying on the prettier plugin for eslint to do code formatting & checking, e.g. in .github/workflows/ci-cd.yml where we invoke yarn lint to check the formatting.

Similarly, the lint:fix script can be used to apply formatting. So the format script is redundant and potentially confusing to new developers.

@github-actions
Copy link
Copy Markdown

@floehopper floehopper force-pushed the remove-broken-format-script branch from 24a61cc to c249a4c Compare April 25, 2024 17:02
@floehopper floehopper temporarily deployed to previews/remove-broken-format-script April 25, 2024 17:02 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown

danhalson
danhalson previously approved these changes Apr 29, 2024
Copy link
Copy Markdown
Contributor

@danhalson danhalson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This script is broken, because no `.prettierrc` file is included in the
repo.

We're relying on the `prettier` plugin for `eslint` to do code
formatting & checking, e.g. in `.github/workflows/ci-cd.yml` where we
invoke `yarn lint` [1] to check the formatting.

Similarly, the `lint:fix` script can be used to apply formatting. So the
`format` script is redundant and potentially confusing to new
developers.

[1]: https://github.com/RaspberryPiFoundation/editor-ui/blob/5197facfc6253be548205d24e89478af286eb384/.github/workflows/ci-cd.yml#L31-L32
@github-actions
Copy link
Copy Markdown

@floehopper
Copy link
Copy Markdown
Contributor Author

LGTM

Thanks, @danhalson. I seem to have lost your review approval by rebasing the branch and fixing the conflicts. Any chance you could re-add it?

@floehopper floehopper merged commit 70ea01c into main Apr 29, 2024
@floehopper floehopper deleted the remove-broken-format-script branch April 29, 2024 08:30
floehopper added a commit that referenced this pull request May 9, 2024
The items were all listed under the "Added" heading, but I'm pretty
confident #976 & #991 were effectively internal fixes that had no impact
on clients of the web component, and it feels like it's worth
highlighting #1003 as a *change* to the existing behaviour rather than
the addition of a new feature.
floehopper added a commit that referenced this pull request May 9, 2024
### Added

- Support to enable embedding iframes in HTML projects from in-house domains (#985)
- Dispatch event when project identifier changes, e.g. after project is remixed (#2830)
- Add `load_remix_disabled` attribute to web component (#992)

### Changed

- Invalidate cached project when project is remixed (#1003)

### Fixed

- Unit tests for `pyodide` runner (#976)
- Remove broken `format` script (#991)
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.

2 participants