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

add alert component #34

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

coder-abdo
Copy link
Contributor

Signed-off-by: abdo-coder [email protected]

Signed-off-by: abdo-coder <[email protected]>
@krancour
Copy link
Contributor

krancour commented Mar 4, 2022

@coder-abdo PR seems to have a lot of unrelated changes in it.

@coder-abdo
Copy link
Contributor Author

I just add two lines of code, the Alert component and importing it

@krancour
Copy link
Contributor

krancour commented Mar 5, 2022

@coder-abdo you may want to double-check that. Maybe your editor is automatically reformatting files somehow when you save? There are a lot of changes in this PR, including the order of important statements, the addition of semicolons to the end of evey statement, and more.

@coder-abdo
Copy link
Contributor Author

I am using prettier as a default formatting package, so I suggest using a formating package like prettier to make the code formatted as standard formating

@krancour
Copy link
Contributor

krancour commented Mar 5, 2022

@coder-abdo the standards we're following currently are consistent with what we've followed elsewhere in the Brigade project. In other places we have a linter incorporated into our CI to validate that PRs adhere to those standards. It's something I'll have in place here as well within the coming week.

If you don't like the standards we currently follow, start a separate discussion about that in its own issue and be clear about what you're proposing and why it's better, and we can have that discussion there.

It's usually bad form to bundle sweeping changes to a project's coding standards into a small, discrete change like this one.

@coder-abdo
Copy link
Contributor Author

what should I do to lint and format my code now?

@krancour
Copy link
Contributor

krancour commented Mar 8, 2022

@coder-abdo I've just opened #37 to automate linting as part of CI.

@krancour
Copy link
Contributor

@coder-abdo are you able to rebase this on the latest from the main branch?

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