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

chore: use reverse dependency tree in oao publish #102

Merged
merged 1 commit into from
Jul 20, 2020
Merged

chore: use reverse dependency tree in oao publish #102

merged 1 commit into from
Jul 20, 2020

Conversation

jeroenptrs
Copy link
Contributor

@jeroenptrs jeroenptrs commented Jun 28, 2020

closes #101

This would allow users to run oao publish --tree so the leaves of a dependency tree get published first

  • add tests

@coveralls
Copy link

coveralls commented Jun 28, 2020

Coverage Status

Coverage increased (+0.2%) to 80.312% when pulling 8a8fcf3 on jeroenptrs:chore/use-tree-in-publish into e3b5ef3 on guigrpa:master.

@jeroenptrs jeroenptrs changed the title [WIP] chore: use reverse dependency tree in oao publish chore: use reverse dependency tree in oao publish Jun 28, 2020
Copy link
Owner

@guigrpa guigrpa left a comment

Choose a reason for hiding this comment

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

This can be very useful, thanks! Please check my comment; otherwise I think it's good to go!

Comment on lines 57 to 61
try {
await publish(merge(NOMINAL_OPTIONS));
} catch (err) {
throw new Error('SHOULD_NOT_HAVE_THROWN');
}
Copy link
Owner

Choose a reason for hiding this comment

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

What's the purpose of this try-catch block? If the publish() fails, the test will already fail…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'll remove it

@jeroenptrs
Copy link
Contributor Author

jeroenptrs commented Jul 19, 2020

@guigrpa pls re-review. Since we're doing --tree by default, I also added a --no-tree flag that disables the dependency graph (if anyone just wants to publish alphabetically)

@guigrpa
Copy link
Owner

guigrpa commented Jul 20, 2020

Hmmm, what would be the added value of the "alphabetical" publishing? If it's not evident, I would remove the flag entirely.

@jeroenptrs
Copy link
Contributor Author

@guigrpa updated PR, please review :)

@jeroenptrs jeroenptrs requested a review from guigrpa July 20, 2020 15:54
@guigrpa
Copy link
Owner

guigrpa commented Jul 20, 2020

Good to go, thanks!

@guigrpa guigrpa merged commit 693e599 into guigrpa:master Jul 20, 2020
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.

Allow --tree in publish command
3 participants