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

refactor: Some QOL changes to improve DX #15

Merged
merged 6 commits into from
Sep 6, 2022
Merged

refactor: Some QOL changes to improve DX #15

merged 6 commits into from
Sep 6, 2022

Conversation

akash1810
Copy link
Member

@akash1810 akash1810 commented Sep 2, 2022

Note
These changes are easiest to review commit by commit.

What does this change?

Make a few QOL improvements to improve the DX:

  • Use departmental config:
  • Repository layout1:
    • Move code files to src directory, this makes it easier to see code vs config etc.
    • Write compiled output to dist directory

How to test

CI should continue to pass.

How can we measure success?

Improved consistency with projects across the department.

Footnotes

  1. These changes could be made in a separate PR to reduce the diff, as GH is showing the changes as delete and recreate rather than file moves.

This project doesn't do anything special, so we can use the default departmental configuration.
To improve consistency with other projects across the department.
To have fewer config files.
This makes it easier to see application code vs config etc.
Comment on lines +56 to +63
deployments: Record<
string,
{
sources?: string[];
[name: string]: unknown;
}
>;
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This has changed from:

deployments: { [name: string]: any }

To provide more type information for index.ts.

Copy link
Contributor

@nicl nicl left a comment

Choose a reason for hiding this comment

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

Looks great! 🚢

If possible, it would be great to use git mv in future to simplify the diff (rather than it thinking everything is deleted/new), though I realise it doesn't always work.

@akash1810
Copy link
Member Author

akash1810 commented Sep 5, 2022

If possible, it would be great to use git mv in future to simplify the diff (rather than it thinking everything is deleted/new), though I realise it doesn't always work.

I had used git mv in the commit, however as changes to the file are made in other commits, I think GitHub interprets the net change as delete+new.

@nicl
Copy link
Contributor

nicl commented Sep 5, 2022

@akash1810 ah yep thanks! It's the linting that is confusing GH.

@akash1810 akash1810 merged commit 12595c6 into main Sep 6, 2022
@akash1810 akash1810 deleted the aa-qol branch September 6, 2022 06:40
@nicl nicl mentioned this pull request Sep 7, 2022
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.

None yet

2 participants