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

Release 1.19 checklist #6469

Closed
12 tasks done
lipis opened this issue Sep 11, 2019 · 29 comments
Closed
12 tasks done

Release 1.19 checklist #6469

lipis opened this issue Sep 11, 2019 · 29 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:meta Issues about the status of Prettier, or anything else about Prettier itself
Milestone

Comments

@lipis
Copy link
Member

lipis commented Sep 11, 2019

Then monitor Twitter/GitHub for any immediate issues 😄

@lipis lipis added this to the 1.19 milestone Sep 11, 2019
@alexander-akait
Copy link
Member

I think will be great update all possible dependencies before do release

@squidfunk
Copy link
Contributor

Any updates on 1.19? When will it be released?

@lydell
Copy link
Member

lydell commented Oct 30, 2019

@squidfunk We’ve finally ticked off all blocking items in the 1.19 milestone!

@jackkoppa
Copy link

jackkoppa commented Oct 31, 2019

Thanks @lydell & awesome work - so happy to see that 🎉

@mrmckeb
Copy link

mrmckeb commented Oct 31, 2019

Is there a way to test this out? I'd be happy to run against a few projects to help test.

@lipis
Copy link
Member Author

lipis commented Oct 31, 2019

Is there a way to test this out? I'd be happy to run against a few projects to help test.

npm install prettier/prettier
# or
yarn add prettier/prettier

@mrmckeb
Copy link

mrmckeb commented Nov 1, 2019

This is working well, thanks @lipis. I'll let you know if we have any issues over the coming days.

@lydell
Copy link
Member

lydell commented Nov 2, 2019

@prettier/core If anyone could help with “Run on other projects to check for regressions” that would be amazing!

@thorn0
Copy link
Member

thorn0 commented Nov 3, 2019

I see a couple of questionable changes like the following. This will surely cause complaints.

Caused by #6229.

Prettier pr-6787
Playground link

--parser typescript

Input:

// ------------------------------------------------print-width----------------->
const response = await this.$http.get<FoooooooooService.DetailsData>(
  `api/foo.ashx/foo-details/${myId}`,
  { cache: this.httpCache, timeout: canceler }
);

Output:

// ------------------------------------------------print-width----------------->
const response = await this.$http.get<
  FoooooooooService.DetailsData
>(`api/foo.ashx/foo-details/${myId}`, {
  cache: this.httpCache,
  timeout: canceler
});

EDIT: This has been fixed in #6791

@lydell
Copy link
Member

lydell commented Nov 3, 2019

@brainkim When running master on some code bases I have access to, I got lots of improvements like this:

       return events;
     }),
     TE.chain(publishEventsToService(dryRun))
-  )().then(E.fold(err => console.error(err), _ => console.log("Went well")));
+  )().then(
+    E.fold(
+      err => console.error(err),
+      _ => console.log("Went well")
+    )
+  );

 ): E.Either<Error, { Bucket: string; Key: string }> =>
   pipe(
-    E.tryCatch(() => new URL(urlString), error => error as Error),
+    E.tryCatch(
+      () => new URL(urlString),
+      error => error as Error
+    ),
     E.map(url => ({

   if (client) {
-    client.close(timeoutMs).then(() => process.exit(1), () => process.exit(1));
+    client.close(timeoutMs).then(
+      () => process.exit(1),
+      () => process.exit(1)
+    );
   } else {

But I also got cases where the output kind of became worse:

 export const timestampOfFirstCommit = (timelines: Timelines): O.Option<Date> =>
-  pipe(
-    timelines,
-    everyCommitTimestamps,
-    A.sort(ordDate),
-    A.head
-  );
+  pipe(timelines, everyCommitTimestamps, A.sort(ordDate), A.head);

 ) => (msg: unknown) =>
   pipe(
     serviceEventFromMessage(msg),
-    TE.chain(
-      flow(
-        publishServiceEvent(analytics),
-        TE.mapLeft(nackFromError)
-      )
-    )
+    TE.chain(flow(publishServiceEvent(analytics), TE.mapLeft(nackFromError)))
   )().then(messageResponse(logger, msg));

   .catch((err: Error) => {
     logger.error(
-      pipe(
-        O.fromNullable(err.stack),
-        O.getOrElse(constant(err.message))
-      )
+      pipe(O.fromNullable(err.stack), O.getOrElse(constant(err.message)))
     );
     process.exit(1);
   });

     expect(
-      pipe(
-        Changelog.timestampOfFirstCommit([[commit]]),
-        O.toUndefined
-      )
+      pipe(Changelog.timestampOfFirstCommit([[commit]]), O.toUndefined)
     ).toStrictEqual(new Date(0));
   });

     ),
-    TE.chain(
-      flow(
-        getUploadUrl,
-        E.mapLeft(Errors.unknownError),
-        TE.fromEither
-      )
-    ),
+    TE.chain(flow(getUploadUrl, E.mapLeft(Errors.unknownError), TE.fromEither)),
     TE.map(url => ({

Thoughts?

EDIT: It’s difficult, though, because I like this change:

-        date_of_incident: pipe(
-          now,
-          subYears(5)
-        ),
+        date_of_incident: pipe(now, subYears(5)),

@brainkim
Copy link
Contributor

brainkim commented Nov 3, 2019

@lydell This is an expected change but I do agree that some of examples become less readable. However, I don‘t think any of these examples are themselves completely unreadable; rather, I believe it‘s just the fact that you’re reading the code in the context of the diff which makes the code feel sub-optimal. Consider:

pipe(timelines, everyCommitTimestamps, A.sort(ordDate), A.head);

English: pipe timelines into everyCommitTimestamps, sort by date and get the head.

pipe(O.fromNullable(err.stack), O.getOrElse(constant(err.message)))
pipe(Changelog.timestampOfFirstCommit([[commit]]), O.toUndefined)

Not sure what these do but I don’t think they’re unreadable.

TE.chain(flow(publishServiceEvent(analytics), TE.mapLeft(nackFromError)))
TE.chain(flow(getUploadUrl, E.mapLeft(Errors.unknownError), TE.fromEither)),

These two seem trickier to read, but I think this has more to do with calling flow within a call to chain. It takes about a second more to figure out whether the arguments passed to flow are passed to flow vs TE.chain, but it’s definitely still doable.

I don’t think any of these changes are show-stoppers, but I clearly cannot be impartial here so it’s up to the core team to decide if these diffs cannot be tolerated. I think trying to split these call expressions onto multiple lines (without the hard-coding behavior from before) would require some more thought. Maybe the rule you want is: when call expressions are nested 3+ levels deep, break the top call onto multiple lines for clarity. I don’t think this is a bad idea, insofar as I have trouble immediately scanning deeply-nested call expressions anyways, but this would have broader repercussions for prettier users.

@j-f1
Copy link
Member

j-f1 commented Nov 4, 2019

I ran it on GitHub Desktop’s (OOP-heavy) codebase and the only changes involved the printing of template strings. Prettier is becoming incredibly stable!

@lydell
Copy link
Member

lydell commented Nov 4, 2019

@j-f1 Good stuff!

@brainkim That makes sense. I think we can ship 1.19 as-is and take it from there.

@lipis
Copy link
Member Author

lipis commented Nov 4, 2019

let's do it.

@pie6k
Copy link

pie6k commented Nov 4, 2019

image

@Domino987

This comment has been minimized.

@thorn0
Copy link
Member

thorn0 commented Nov 5, 2019

1.18.2 can format code like this:

class A {} class A {}

but master can't because Babel became more restrictive:

SyntaxError: Identifier 'A' has already been declared (1:18)

This looks like a breaking change, so before doing the release, it might make sense to try updating Babel to 7.7.0 and enabling error recovery.

@alexander-akait
Copy link
Member

@thorn0 good catch 👍

@thorn0
Copy link
Member

thorn0 commented Nov 5, 2019

addressed in #6816

@hlhr202
Copy link

hlhr202 commented Nov 6, 2019

cant wait!! GJ!!

@EdenTurgeman
Copy link

Can't wait for full TS 3.7 support!

@brodybits
Copy link
Contributor

I just raised PR #6853 that resolves a critical yarn audit issue, hope it can be part of the upcoming release. With the update from PR #6853, I verified that all other yarn audit issues are on devDependencies.

@waterfoul
Copy link

waterfoul commented Nov 7, 2019

Any status or ETA on when the release could happen? Anything anyone can do to help make the release happen?

@alexander-akait
Copy link
Member

Tomorrow (Friday)

@kbradl16
Copy link

kbradl16 commented Nov 8, 2019

Tomorrow (Friday)

What time are we expecting this to be released?

@lipis
Copy link
Member Author

lipis commented Nov 8, 2019

it's still Friday in some places :)

@lydell
Copy link
Member

lydell commented Nov 8, 2019

I tried to make the release today, but couldn’t get the release script to work. Will have to try again tomorrow. 😢

@buko106
Copy link

buko106 commented Nov 9, 2019

prettier@1.19.0 is out now!! thanks!!

@lydell
Copy link
Member

lydell commented Nov 9, 2019

Yeah, I finally managed to find a workaround… 😅

@lydell lydell closed this as completed Nov 9, 2019
@lydell lydell unpinned this issue Nov 9, 2019
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Feb 7, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 7, 2020
@thorn0 thorn0 added the type:meta Issues about the status of Prettier, or anything else about Prettier itself label Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:meta Issues about the status of Prettier, or anything else about Prettier itself
Projects
None yet
Development

No branches or pull requests