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

Frontend assets are outdated #15726

Closed
Piedone opened this issue Apr 10, 2024 · 8 comments · Fixed by #15735
Closed

Frontend assets are outdated #15726

Piedone opened this issue Apr 10, 2024 · 8 comments · Fixed by #15735
Assignees
Labels
Milestone

Comments

@Piedone
Copy link
Member

Piedone commented Apr 10, 2024

Describe the bug

The assets_validation workflow didn't catch this, because it has a bug, but if you build the frontend assets then there will be changed files. This means that when you run OC, the client-side assets are actually outdated.

See a correctly failing assets_validation here.

Once #15619 is merged, assets_validation will be fixed.

To Reproduce

In the root of the repo:

  1. npm install
  2. gulp rebuild

Expected behavior

No files changed.

Also, we should run assets_validation as a CI workflow too, this shouldn't be just on-demand. So, add the following triggers to it:

on: 
  push:
    paths-ignore:
      - '**/*.md'
      - 'mkdocs.yml'
      - 'src/docs/**/*'
    branches: [ main ]
  pull_request:
    branches: [ main, release/** ]
concurrency:
  group: ${{ github.head_ref || github.run_id }}
  cancel-in-progress: true

Screenshots

image

@Piedone
Copy link
Member Author

Piedone commented Apr 10, 2024

Is this maybe something you'd be interested in, @agriffard?

@Piedone Piedone self-assigned this Apr 11, 2024
@BenedekFarkas BenedekFarkas changed the title Frontend assets our outdated Frontend assets are outdated Apr 11, 2024
@BenedekFarkas
Copy link
Member

Is the workflow intentionally not using paths instead of paths-ignore? I just added a similar workflow job in O1 and I didn't add any filtering there yet, but for O1 it would probably be just **.less and **.js, OC might be more complicated.

@Piedone
Copy link
Member Author

Piedone commented Apr 11, 2024

I just copied that from the Main workflow, but it looks correct: we want the workflow to run on all main pushes except if the commit only changed the docs, since then it's pointless.

@MikeAlhayek
Copy link
Member

@Piedone I am going over all the tagged PR with milestone 1.9. Why would this issue block 1.9? I mean we should be able to ship 1.9 without this issue, right?

I think this one should be tagged with 1.x so it does not block 1.9. Thoughts?

@Piedone
Copy link
Member Author

Piedone commented Apr 15, 2024

Currently, we don't know how much the C# and Razor code is not in sync with JS and CSS. Bugs that are supposed to be fixed might not work, or we might have code with the opposite in the source files and the asset files (we've seen that).

So I don't think we should ship 1.9 without this, no.

@Piedone
Copy link
Member Author

Piedone commented Apr 15, 2024

BTW I went through the recent bug issues without a milestone, looking for regressions since 1.8 (because those we should all fix before 1.9) but couldn't find any new ones.

@MikeAlhayek
Copy link
Member

Once #15619 is merged, assets_validation will be fixed.

That referenced PR was merged. Do we still need to run gulp rebuild on a new branch and update all of out assets manually at least one time to catch up anything that was missed?

@Piedone
Copy link
Member Author

Piedone commented Apr 15, 2024

That PR merged the assets_validation GitHub Actions workflow. So, it could now tell us that asset validation fails :). We still need to fix the assets, yes. This issue is about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants