Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

Rename lib to utils #2153

Merged
merged 5 commits into from
Dec 10, 2019
Merged

Rename lib to utils #2153

merged 5 commits into from
Dec 10, 2019

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Dec 6, 2019

Rename all lib folders to utils so we can gitignore lib and use it for build output. (I could use utilities or some other name instead if people prefer.)

Another option is to move the things formerly under packages/react/src/lib to their own package, but that would require significantly more work/breaks.

I did NOT actually add lib to .gitignore yet because I'm concerned that would cause any changes to lib in open PRs to just disappear when they merge with master (instead of being merged into the moved files).

The following open PRs touch files in lib folders: #2115, #2044, #2006, probably #1999, #1991, #1980, #1867, #1301. Other PRs may also add/modify references to lib imports, but this will be caught by a merge conflict and/or build error.

Microsoft Reviewers: Open in CodeFlow

@@ -1,10 +1,8 @@
CONTRIBUTING
============
# CONTRIBUTING
Copy link
Member Author

Choose a reason for hiding this comment

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

The formatting changes here are because I have my editor set up to run prettier on save (could revert if desired). Same in a couple other markdown files.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should revert this - danger actually checks for diffs. So this will blow up danger.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not quite sure what you mean about danger?

Copy link
Member

@kenotron kenotron left a comment

Choose a reason for hiding this comment

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

I think the only thing I want to mention is to preserve the CHANGELOG.md formatting for now - maybe we should have added CHANGELOG.md in .prettierignore

@@ -1,10 +1,8 @@
CONTRIBUTING
============
# CONTRIBUTING
Copy link
Member

Choose a reason for hiding this comment

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

I think we should revert this - danger actually checks for diffs. So this will blow up danger.

build/dangerjs/checkChangelog.ts Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### BREAKING CHANGES
- Rename `lib` directories to `utils` @ecraig12345 ([#2153](https://github.com/microsoft/fluent-ui-react/pull/2153))
Copy link
Member

Choose a reason for hiding this comment

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

Not sure that this is a really breaking change as I don't see anything that can affect consumer that uses public API.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's good to know, thank you! Some of my experience with Fabric has led me to be very cautious about assuming internal path changes are non-breaking, but if you think it's safe here that's great.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants