Skip to content

Deprecate the filename option #42

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

Closed
wants to merge 1 commit into from
Closed

Deprecate the filename option #42

wants to merge 1 commit into from

Conversation

lydell
Copy link

@lydell lydell commented Jun 2, 2014

Why?

  • It is only used to set the file property of the source map, and
    that’s probably done just because the file property used to be a
    required property of source maps. It isn’t no more.
  • The file property is not used by browsers.
  • The user could easily just add the property himself on the returned
    source map object.
  • It was neither documented nor tested.
  • KISS.

Why?

- It is only used to set the `file` property of the source map, and
  that’s probably done just because the `file` property used to be a
  required property of source maps. It isn’t no more.
- The `file` property is not used by browsers.
- The user could easily just add the property himself on the returned
  source map object.
- It was neither documented nor tested.
- KISS.
@conradz
Copy link
Contributor

conradz commented Jun 2, 2014

I think if we decide to deprecate filename, we can just remove it altogether and bump major version.

@jonathanong
Copy link
Contributor

@lydell yo i noticed a lot of PRs you've made and appreciate them. @necolas is considering merging these two libraries in to css so maintenance would be easier: reworkcss/css#13. i think doing all this source map stuff after we merged these libraries would be easier.

@conradz
Copy link
Contributor

conradz commented Jun 2, 2014

@jonathanong woops, hadn't noticed that issue, 👍 on it anyways.

@necolas
Copy link
Contributor

necolas commented Jun 2, 2014

I'm happy to merge the libraries whenever it's easiest for everyone else. I was thinking we'd do it once things stabilized just before new releases, but if you'd prefer to merge them before finalizing the source mapping, that's cool too. lmk

@jonathanong
Copy link
Contributor

Doesn't matter to me, I just think it might be easier to merge them now

@conradz
Copy link
Contributor

conradz commented Jun 3, 2014

Ya, I think it might be better to merge now. No big deal either way though.

@lydell
Copy link
Author

lydell commented Jun 3, 2014

I think if we decide to deprecate filename, we can just remove it altogether and bump major version.

Ok, closing this then.


It doesn’t matter to me either. There’s no rush merging any PRs, and I could submit new ones after the merge if needed.

@lydell lydell closed this Jun 3, 2014
@necolas
Copy link
Contributor

necolas commented Jun 4, 2014

OK I've merged things into reworkcss/css. Feel free to develop against that and port over any existing issues.

lydell added a commit to lydell/css that referenced this pull request Jun 4, 2014
Why?

- It is only used to set the `file` property of the source map, and
  that’s probably done just because the `file` property used to be a
  required property of source maps. It isn’t no more.
- The `file` property is not used by browsers.
- The user could easily just add the property himself on the returned
  source map object.
- It was neither documented nor tested.
- KISS.

@conradz:

> I think if we decide to deprecate filename, we can just remove it
> altogether and bump major version.

reworkcss/css-stringify#42 (comment)
@lydell
Copy link
Author

lydell commented Jun 4, 2014

reworkcss/css#26

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.

4 participants