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

fix: generate source maps using sass with asset/resource #1010

Closed
wants to merge 2 commits into from
Closed

Conversation

xoxys
Copy link

@xoxys xoxys commented Jan 8, 2022

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

Supersedes: #968
Fixes: #962

Breaking Changes

No

Additional Info

No

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 8, 2022

CLA Signed

The committers are authorized under a signed CLA.

@xoxys xoxys changed the title feat: generate source maps using sass with asset/resource fix: generate source maps using sass with asset/resource Jan 8, 2022
@xoxys
Copy link
Author

xoxys commented Jan 8, 2022

@alexander-akait How to handle the CLA? I mean, the commit was not done by me...

@alexander-akait
Copy link
Member

You should not use another person PR, just send the same, CLA was assign by Alexey Lavinsky so you have right to copy code

@xoxys
Copy link
Author

xoxys commented Jan 8, 2022

Why is my mailing address required to for the CLA? IMO a bit too much data that is requested.

@alexander-akait
Copy link
Member

We use email from commits

@xoxys
Copy link
Author

xoxys commented Jan 10, 2022

No Im talking about my postal address. The docusign document that need to be signed during the CLA process is asking for my private mailing/postal address...

@alexander-akait
Copy link
Member

Very strange, it never asks it before, are you assign as Individual Contributor?

@xoxys
Copy link
Author

xoxys commented Jan 10, 2022

Yes as an individual contributor:
image

@alexander-akait
Copy link
Member

Just use the same email as you use in commit

@xoxys
Copy link
Author

xoxys commented Jan 10, 2022

It is not the email address, there is a dedicated field for it, not shown in the screenshot. However, I've added private to the two lines now, hope that works for you

@xoxys
Copy link
Author

xoxys commented Jan 10, 2022

How to proceed? Workflows need approvals I guess.

@xoxys
Copy link
Author

xoxys commented Jan 11, 2022

@alexander-akait looks like it needs another approval

@codecov
Copy link

codecov bot commented Jan 11, 2022

Codecov Report

Merging #1010 (d560a2e) into master (babe42a) will increase coverage by 0.05%.
The diff coverage is 97.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1010      +/-   ##
==========================================
+ Coverage   95.91%   95.97%   +0.05%     
==========================================
  Files           5        5              
  Lines         294      323      +29     
  Branches       97      113      +16     
==========================================
+ Hits          282      310      +28     
- Misses         11       12       +1     
  Partials        1        1              
Impacted Files Coverage Δ
src/index.js 97.43% <92.30%> (-2.57%) ⬇️
src/utils.js 95.86% <100.00%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update babe42a...d560a2e. Read the comment docs.

@alexander-akait
Copy link
Member

There is interesting situation around #774, new compiler API fully remove all related source map options (i.e. only sourceMap: true) exists, so looking ahead I think we need to revisit something here, I think we should allow to generate custom sourceMappingURL, otherwise the current solution will not be compatible with modern sass API. Any ideas?

@xoxys
Copy link
Author

xoxys commented Jan 11, 2022

I think this goes a bit beyond my knowledge about how all works together :(

@alexander-akait
Copy link
Member

yep, hard to find better solution here, only developer know location, maybe we should allow to modify CSS from Sass...

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.

unable to generate sourcemap with asset/resource
2 participants