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.

Sign up for GitHub

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

Jump to bottom

[3/6] build: stop suffixing XModule SCSS with hashes #32288

Merged
merged 1 commit into from
Jun 7, 2023
Merged

[3/6] build: stop suffixing XModule SCSS with hashes #32288

merged 1 commit into from
Jun 7, 2023

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented May 23, 2023 •
edited
Loading

Supporting information

This is a part of a series of PRs:

The previous PR is:

The next PR is:

Description

Similar to #32287,
this change removes another unnecessary step from the
xmodule_assets script. The script, which generates XModule
SCSS "entrypoint" files (synthesizing one or more "source" SCSS
resources), was appending MD5 hashes to each SCSS entrypoint filename:

common/static/xmodule/descriptors/scss:
   AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
   AnnotatableBlockStudio.be69909d83985d31e206fad272906958.scss
   ConditionalBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
   CourseInfoBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
   CustomTagBlockStudio.be69909d83985d31e206fad272906958.scss
   HtmlBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
   LibraryContentBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
   LTIBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
   PollBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
   ProblemBlockStudio.5893b30426f88e14712556c6c4342f23.scss
   SequenceBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
   SplitTestBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
   StaticTabBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
   VideoBlockStudio.e4a6920a875dfb91eb65ee7e6dad7e2e.scss
   WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
common/static/xmodule/modules/scss:
   AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
   AnnotatableBlockPreview.7e95b106aa0a61824f4290da1374960d.scss
   ConditionalBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
   CourseInfoBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
   CustomTagBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
   HtmlBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
   LibraryContentBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
   LTIBlockPreview.a763928b2c415251720f8634b8daee59.scss
   PollBlockPreview.39730e54c1eebbafd18a82fbb09c1e37.scss
   ProblemBlockPreview.70b905ac161108a0a03c639232450aaa.scss
   SequenceBlockPreview.e2336fa64ba495fa7c0f4f838d20ad8c.scss
   SplitTestBlockPreview.d41d8cd98f00b204e9800998ecf8427e.scss
   StaticTabBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
   VideoBlockPreview.b2de5e1c4da8cba16ce1c4bdeab50f9e.scss
   WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss

It's unclear why these MD5 hashes needed to be appended.
A comment in xmodule/static_content.py hints that it might have
something to do with de-duplication, but that doesn't make any sense,
because each XModule has exactly two SCSS entrypoint files (one for
studio_view and one for other student/author_views) and none of those
entrypoint files can possibly be shared between XModules.

Soon, as part of deleting the xmodule_assets script,
we would like to just check these SCSS files into version control
rather than generating them. In order to do that, we will need to
drop the hashes. This PR does that.
The new output looks like this:

common/static/xmodule/descriptors:
   AboutBlockStudio.scss
   AnnotatableBlockStudio.scss
   ConditionalBlockStudio.scss
   CourseInfoBlockStudio.scss
   CustomTagBlockStudio.scss
   HtmlBlockStudio.scss
   LibraryContentBlockStudio.scss
   LTIBlockStudio.scss
   PollBlockStudio.scss
   ProblemBlockStudio.scss
   SequenceBlockStudio.scss
   SplitTestBlockStudio.scss
   StaticTabBlockStudio.scss
   VideoBlockStudio.scss
   WordCloudBlockStudio.scss
common/static/xmodule/modules:
   AboutBlockPreview.scss
   AnnotatableBlockPreview.scss
   ConditionalBlockPreview.scss
   CourseInfoBlockPreview.scss
   CustomTagBlockPreview.scss
   HtmlBlockPreview.scss
   LibraryContentBlockPreview.scss
   LTIBlockPreview.scss
   PollBlockPreview.scss
   ProblemBlockPreview.scss
   SequenceBlockPreview.scss
   SplitTestBlockPreview.scss
   StaticTabBlockPreview.scss
   VideoBlockPreview.scss
   WordCloudBlockPreview.scss

Testing Instructions

  • Build the Tutor openedx image with this branch
  • Browse the demo course in the Studio outline editor
  • Browse the demo course in Learning MFE

Deadline

Medium-high urgency, as this is in the critical path to a long line of DevX improvement PRs.

This was referenced May 23, 2023
Merged
Closed
@kdmccormick kdmccormick changed the title Kdmccormick/xmodule scss hashes build: stop suffixing XModule SCSS with hashes May 23, 2023
@kdmccormick kdmccormick mentioned this pull request May 23, 2023
Merged
@kdmccormick kdmccormick changed the title build: stop suffixing XModule SCSS with hashes [3] build: stop suffixing XModule SCSS with hashes May 23, 2023
@kdmccormick kdmccormick changed the title [3] build: stop suffixing XModule SCSS with hashes [3/6] build: stop suffixing XModule SCSS with hashes May 23, 2023
This was referenced May 23, 2023
Merged
Closed
@kdmccormick
build: stop suffixing XModule SCSS with hashes
9c873e1 
Similar to openedx#32287,
this change removes another unnecessary step from the
`xmodule_assets` script. The script, which generates XModule
SCSS "entrypoint" files (synthesizing one or more "source" SCSS
resources), was appending MD5 hashes to each SCSS entrypoint filename:

    common/static/xmodule/descriptors/scss:
       AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
       ...
       WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
    common/static/xmodule/modules/scss:
       AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
       ...
       WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss

It's unclear why these MD5 hashes needed to be appended.
A comment in xmodule/static_content.py hints that it might have
something to do with de-duplication, but that doesn't make any sense,
because each XModule has exactly two SCSS entrypoint files (one for
studio_view and one for other student/author_views) and none of those
entrypoint files can possibly be shared between XModules.

Soon, as part of deleting the `xmodule_assets` script,
we would like to just check these SCSS files into version control
rather than generating them. In order to do that, we will need to
drop the hashes. This commit does that.
The new output looks like this:

    common/static/xmodule/descriptors:
       AboutBlockStudio.scss
       ...
       WordCloudBlockStudio.scss
    common/static/xmodule/modules:
       AboutBlockPreview.scss
       ...
       WordCloudBlockPreview.scss

Part of: openedx#32292
@kdmccormick kdmccormick force-pushed the kdmccormick/xmodule-scss-hashes branch from 97465e3 to 9c873e1 Compare June 6, 2023 13:33
@kdmccormick kdmccormick marked this pull request as ready for review June 6, 2023 13:33
andrey-canon
andrey-canon approved these changes Jun 7, 2023
View reviewed changes
Copy link
Contributor

@andrey-canon andrey-canon left a comment

Choose a reason for hiding this comment

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

LGTM, I thought this was going to affect the cache but I performed some tests and everything works normally

@kdmccormick kdmccormick merged commit cef8c06 into openedx:master Jun 7, 2023
@kdmccormick kdmccormick deleted the kdmccormick/xmodule-scss-hashes branch June 7, 2023 13:26
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

kdmccormick added a commit that referenced this pull request Jun 7, 2023
@kdmccormick
Revert "build: stop suffixing XModule SCSS with hashes (#32288)"
d05931a 
This reverts commit cef8c06.
@kdmccormick kdmccormick mentioned this pull request Jun 7, 2023
Closed
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

Yagnesh1998 pushed a commit to ManpraXSoftware/edx-platform that referenced this pull request Jun 8, 2023
@kdmccormick
build: stop suffixing XModule SCSS with hashes (openedx#32288)
d4cad46 
Similar to openedx#32287,
this change removes another unnecessary step from the
`xmodule_assets` script. The script, which generates XModule
SCSS "entrypoint" files (synthesizing one or more "source" SCSS
resources), was appending MD5 hashes to each SCSS entrypoint filename:

    common/static/xmodule/descriptors/scss:
       AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
       ...
       WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
    common/static/xmodule/modules/scss:
       AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
       ...
       WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss

It's unclear why these MD5 hashes needed to be appended.
A comment in xmodule/static_content.py hints that it might have
something to do with de-duplication, but that doesn't make any sense,
because each XModule has exactly two SCSS entrypoint files (one for
studio_view and one for other student/author_views) and none of those
entrypoint files can possibly be shared between XModules.

Soon, as part of deleting the `xmodule_assets` script,
we would like to just check these SCSS files into version control
rather than generating them. In order to do that, we will need to
drop the hashes. This commit does that.
The new output looks like this:

    common/static/xmodule/descriptors:
       AboutBlockStudio.scss
       ...
       WordCloudBlockStudio.scss
    common/static/xmodule/modules:
       AboutBlockPreview.scss
       ...
       WordCloudBlockPreview.scss

Part of: openedx#32292
Yagnesh1998 pushed a commit to ManpraXSoftware/edx-platform that referenced this pull request Jun 8, 2023
@kdmccormick
build: stop suffixing XModule SCSS with hashes (openedx#32288)
69e5d1e 
Similar to openedx#32287,
this change removes another unnecessary step from the
`xmodule_assets` script. The script, which generates XModule
SCSS "entrypoint" files (synthesizing one or more "source" SCSS
resources), was appending MD5 hashes to each SCSS entrypoint filename:

    common/static/xmodule/descriptors/scss:
       AboutBlockStudio.768623f4d8d73dfb637fc94583adb990.scss
       ...
       WordCloudBlockStudio.d41d8cd98f00b204e9800998ecf8427e.scss
    common/static/xmodule/modules/scss:
       AboutBlockPreview.05a6cbd5c10100a245fa2cbf151b9770.scss
       ...
       WordCloudBlockPreview.7b899a56a70d29c58cf14b7e1888a0ec.scss

It's unclear why these MD5 hashes needed to be appended.
A comment in xmodule/static_content.py hints that it might have
something to do with de-duplication, but that doesn't make any sense,
because each XModule has exactly two SCSS entrypoint files (one for
studio_view and one for other student/author_views) and none of those
entrypoint files can possibly be shared between XModules.

Soon, as part of deleting the `xmodule_assets` script,
we would like to just check these SCSS files into version control
rather than generating them. In order to do that, we will need to
drop the hashes. This commit does that.
The new output looks like this:

    common/static/xmodule/descriptors:
       AboutBlockStudio.scss
       ...
       WordCloudBlockStudio.scss
    common/static/xmodule/modules:
       AboutBlockPreview.scss
       ...
       WordCloudBlockPreview.scss

Part of: openedx#32292
NawfalAhmed added a commit that referenced this pull request Jun 14, 2023
@NawfalAhmed
Revert "build: stop suffixing XModule SCSS with hashes (#32288)"
e00dcfb 
This reverts commit cef8c06.
kdmccormick added a commit to kdmccormick/edx-platform that referenced this pull request Jun 16, 2023
@kdmccormick
build: commit XModule SCSS entrypoints of generating them
b93e41d 
`xmodule_assets` generated a series of SCSS "entrypoint"
files, where each entrypoint file imported from the
SCSS "sources" in xmodule/css.

This process was more complicated up until very
recently; for context, make sure you've seen:
 * openedx#32288
 * openedx#32289

Now that the process is simpler, though, there is no
reason to generate the SCSS entrypoints; we can just
commit them to the repository instead!
So, we go from this:

    # GENERATED: SCSS entrypoints files for CMS
    common/static/xmodule/descriptors:
       AboutBlockStudio.scss
       AnnotatableBlockStudio.scss
       ...
    # GENERATED: SCSS entrypoints files for LMS
    common/static/xmodule/modules:
       AboutBlockPreview.scss
       AnnotatableBlockPreview.scss
       ...
    # VERSION CONTROLLED: SCSS source files
    xmodule/css:
      annotatable/...
      capa/...
      ...

to this:

    # VERSION CONTROLLED: All XModule SCSS
    xmodule/static/sass:
      # Source files
      include:
        annotatable/...
        capa/...
        ...
      # CMS entrypoint files
      cms:
        AboutBlockStudio.scss
        AnnotatableBlockStudio.scss
        ...
      # LMS source files
      lms:
        AboutBlockPreview.scss
        AnnotatableBlockPreview.scss
        ...

Also, we are able to remove all SCSS-related logic from the
`xmodule_assets` script and from the `HTMLSnippet` class.
XModule JS assets still need processing, but we will address
those in a separate series of PRs.

Part of: openedx#32292
@kenclary kenclary mentioned this pull request Jun 29, 2023
Merged
@kenclary kenclary mentioned this pull request Aug 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrey-canon andrey-canon andrey-canon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kdmccormick @edx-pipeline-bot @andrey-canon