Navigation Menu

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

Ensuring environment.ts is present before sync-i18n-files invocation #676

Merged
merged 1 commit into from May 18, 2020

Conversation

bram-atmire
Copy link
Member

References

Description

The sync-i18n-files script fails if the person executing it has never executed "yarn run config", causing environment.ts to be absent

Instructions for Reviewers

Observations pre-fix: executing "yarn run sync-i18n" produces the error Error: Cannot find module '../src/environments/environment.ts'

Observations post-fix: sync-18n works again

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes TSLint validation using yarn run lint
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs for any bug fixes, improvements or new features. A few reminders about what constitutes good tests:
    • Include tests for different user types (if behavior differs), including: (1) Anonymous user, (2) Logged in user (non-admin), and (3) Administrator.
    • Include tests for error scenarios, e.g. when errors/warnings should appear (or buttons should be disabled).
    • For bug fixes, include a test that reproduces the bug and proves it is fixed. For clarity, it may be useful to provide the test in a separate commit from the bug fix.
  • If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

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

Thanks @bram-atmire!

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

Looks good to me too

@tdonohue tdonohue merged commit b28f59f into DSpace:master May 18, 2020
@tdonohue tdonohue added this to Needs Reviewers Assigned in DSpace 7 Beta 3 via automation Jun 30, 2020
@tdonohue tdonohue moved this from Needs Reviewers Assigned to Done in DSpace 7 Beta 3 Jun 30, 2020
@tdonohue tdonohue added this to the 7.0beta3 milestone Jun 30, 2020
4science-it pushed a commit to 4Science/dspace-angular that referenced this pull request Jun 23, 2023
DSC-1140

Approved-by: Giuseppe Digilio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants