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 support for Flags #41

Merged
merged 12 commits into from
Nov 1, 2018
Merged

add support for Flags #41

merged 12 commits into from
Nov 1, 2018

Conversation

damiencourousse
Copy link
Contributor

@damiencourousse damiencourousse commented Oct 31, 2018

This pull request supports issue #8. It adds the following changes:

  • add hard-coded flags FIXME and XXX
  • add support for user-defined flags. The flags can be defined in the config file and/or from the CLI (option -u).
  • show the flag type in the UI.

Help needed:
Since the merge with master, the delete function no longer works. I am not fluent with JavaScript and need your help to debug this. While merging with master, I spotted probably bugous behaviours in the UI. See issues #39 and #40. The "deletion problem" in this branch could be related to those issues, or not.

@damiencourousse
Copy link
Contributor Author

damiencourousse commented Oct 31, 2018 via email

Copy link
Owner

@aviaviavi aviaviavi left a comment

Choose a reason for hiding this comment

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

This is a great change, just 1 point of feedback. Sorry for all the merge conflicts from that previous refactor by the way.

app/Config.hs Show resolved Hide resolved
@damiencourousse
Copy link
Contributor Author

damiencourousse commented Nov 1, 2018 via email

@damiencourousse
Copy link
Contributor Author

damiencourousse commented Nov 1, 2018 via email

Copy link
Owner

@aviaviavi aviaviavi 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, thank you!

@aviaviavi aviaviavi merged commit 2b60c23 into aviaviavi:master Nov 1, 2018
@aviaviavi
Copy link
Owner

If you don't mind, please could you do this change? I am not a web
developer, and I am not sure how to do this :-[.

Not at all, will do that. :) I actually had already deleted that comment when I remembered you mentioned this

@damiencourousse
Copy link
Contributor Author

damiencourousse commented Nov 1, 2018 via email

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.

2 participants